Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance degradation for indexed string property access #719

Closed
skinny85 opened this issue Apr 3, 2023 · 4 comments
Closed

Performance degradation for indexed string property access #719

skinny85 opened this issue Apr 3, 2023 · 4 comments
Assignees
Labels
performance Performance of the engine (peak or warmup)

Comments

@skinny85
Copy link

skinny85 commented Apr 3, 2023

First of all, thank you for all the work you do on Graal JS and Graal itself, it's an awesome project 馃檪.

I authored of a series of blog posts that aim to be an introduction to working with Truffle and GraalVM. While working on the latest chapter on strings, I've ran into some interesting numbers for a simple benchmark.

The idea is to count to n in a loop, while using some simple string operations. The benchmark has two variants, one with direct property access:

function countWhileCharAtDirectProp(n) {
    var ret = 0;
    while (n > 0) {
        n = n - ('a'.charAt(0) + ''.charAt()).length;
        ret = ret + 1;
    }
    return ret;
}

And the other uses indexed property access:

function countWhileCharAtIndexProp(n) {
    var ret = 0;
    while (n > 0) {
        n = n - ('a'['charAt'](0) + ''['charAt']())['length'];
        ret = ret + 1;
    }
    return ret;
}

The benchmark invokes the function with n equal to 1_000_000.

Here are the results on my laptop (ezs is the toy implementation developed in the article series, called EasyScript) - the units are microseconds per iteration, so lower is better:

Benchmark                                                  Mode  Cnt       Score      Error  Units
StringLengthBenchmark.count_while_char_at_direct_prop_ezs  avgt    5     576.093 卤    5.992  us/op
StringLengthBenchmark.count_while_char_at_direct_prop_js   avgt    5     576.772 卤    3.865  us/op
StringLengthBenchmark.count_while_char_at_index_prop_ezs   avgt    5     576.813 卤    7.087  us/op
StringLengthBenchmark.count_while_char_at_index_prop_js    avgt    5  112404.250 卤 1012.309  us/op

So, the "index access" variant is 200 times slower than the "direct access" variant.

I thought you'd be interested in seeing this - I assume this is a simple bug in the implementation 馃檪.

The version of GraalVM used:

$ java -version

openjdk version "17.0.5" 2022-10-18
OpenJDK Runtime Environment GraalVM CE 22.3.0 (build 17.0.5+8-jvmci-22.3-b08)
OpenJDK 64-Bit Server VM GraalVM CE 22.3.0 (build 17.0.5+8-jvmci-22.3-b08, mixed mode, sharing)

And org.graalvm.js:js is in the same version (22.3.0).


Let me know if you have any questions!

Thanks,
Adam

@iamstolis iamstolis self-assigned this Apr 3, 2023
@iamstolis iamstolis added the performance Performance of the engine (peak or warmup) label Apr 3, 2023
@iamstolis
Copy link
Member

iamstolis commented Apr 4, 2023

Thank you for the report. There was really a simple optimization opportunity in our implementation of reads of the form primitiveValue[key]. I can see a comparable performance of countWhileCharAtDirectProp and countWhileCharAtIndexProp with d4442d6.

@skinny85
Copy link
Author

skinny85 commented Apr 5, 2023

Thanks for the quick fix @iamstolis! Any idea when, and in what version, it will be released?

@iamstolis
Copy link
Member

It hasn't made it into GraalVM 23.0.0 that we are about to release. So, it will be in GraalVM 23.1.0. You can use/test a development build in the meantime. The fix should appear there in a week or so.

@skinny85
Copy link
Author

Thank you so much @iamstolis!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance of the engine (peak or warmup)
Projects
None yet
Development

No branches or pull requests

2 participants