Bug 210135 – [JSC][LLInt] Cache subscript for get_by_val operations

• Component: JavaScriptCore
• Status: NEW

[Caio Lima]

Currently, we are saving seen identifiers into `get_by_val` and use such information to decide if it is worth it add IC for a given `get_by_val`. However, there is no IC support for cacheable cells on LLInt yet.

[Caio Lima]

I have the following results collected so far regarding memory usage and performance. Those results were collected using a MacBook Pro 15' late 2018.

RAMification score (T-test considering 95% confidence interval)

Mean of 10 runs ToT: 30806174.9 +- 60805.43
Mean of 10 runs Changes: 30803689.0 +- 61224.86
This indicates that those changes have neutral impact on memory usage.

------------------------------------------------------------------------

Speedometer 2 (T-test considering 95% confidence interval)

Mean of 50 iterations ToT: 117.21599999999998 +- 0.71
Mean of 50 iterations Changes: 118.772 +- 0.77

This indicates a progression on Speedometer.

------------------------------------------------------------------------

JetStream 2 (T-test considering 95% confidence interval)

Mean of 5 runs ToT: 157.7564 +- 2.99
Mean of 5 runs Changes: 157.14499999999998 +- 1.92

This indicates that changes are performance neutral

[Caio Lima]

Created attachment 395738 [details]
WIP - Patch

[Filip Pizlo]

Comment on attachment 395738 [details]
WIP - Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=395738&action=review

I think that what you need to land this patch is to write some tests.

I recommend:

1) microbenchmark that is cooked to demonstrate maximum speed-up.  We can use this test to eyeball whether the optimization works.  You might want to do evil stuff like have an eval that creates a new version every 100 iterations so that it never gets to the JIT but does repeat enough each time to benefit from ICs. Aim to have it run for <=100ms on a decent machine.

2) tests for all of the important conditions.  You've got to test what happens when you ping-pong the IC, for example.  We surely have tests like this for JIT versions of this IC, but you want to write specific versions for your change and verify that if you break your change, the tests fail.

I would expect to see 5ish tests (maybe 1 microbenchmark and 4 stress tests) for a change like this.

> Source/JavaScriptCore/ChangeLog:14
> +        We are introducing in this patch self property access IC for
> +        `get_by_val` on LLInt. For this new case, we need to cache StructureID,
> +        property offset and subscript. This change makes `get_by_val` faster on
> +        cases where access is monomorphic both on receiver and subscript.
> +        The cached information is not being used by JIT tiers to improve
> +        generated code, but we can ivestigate this in the future. Results of
> +        memory usage and performance improvements are listed bellow:

This is a really good idea.

[Radar WebKit Bug Importer]

<rdar://problem/96305341>