Closed Bug 1180772 Opened 9 years ago Closed 16 days ago

IonMonkey: GetElem ICs should atomize incoming keys

Categories

(Core :: JavaScript Engine: JIT, defect, P5)

defect

Tracking

()

RESOLVED DUPLICATE of bug 1867359

People

(Reporter: djvj, Unassigned)

References

(Blocks 1 open bug)

Details

IonMonkey's GetElem ICs have a different way of handling non-atomized incoming property names than baseline.

If an incoming proprety name is not atomized, Ion calls out to a slowpath that does a string comparison between the name and the key associated with the cache.  Baseline instead atomizes the name in a slow-path and then compares the newly atomized name directly with the key using a ptr-equality.

THe reasoning behind the baseline behaviour is that the property name will be pre-atomized for any subsequent stubs it hits (if the current stub is not a match).  Furthermore, if a name gets used multiple times in the same source location (e.g. a loop) it'll be atomized for all subsequent lookups and the slowpath can be avoided.

With Ion's current approach, a non-atomized name will always keep taking the slowpath string comparison when traveling through Ion ICs.

Suggestion is to change Ion ICs to do the baseline behaviour - atomize incoming property names if they're not already atomized, and use ptr-equality to compare keys.
Blocks: sm-js-perf
Priority: -- → P5
Did this got fixed in the last few weeks?
Given the description I would say this is bug 1324566 ?
Flags: needinfo?(jdemooij)
I think the CacheIR stubs are using Ion's behavior atm. We could do measurements at some point to see if atomizing in EqualStringsHelper is a win.
Flags: needinfo?(jdemooij)
Severity: normal → S3

A lot of code has changed since this bug was opened. A GetElem IC with a non-constant string key will end up in the megamorphic property code, which always atomizes. Also, the optimization we added in bug 1867359 will eagerly atomize strings used as property keys when we load them out of an object slot, which avoids having to atomize the same string repeatedly. If the string is a constant, but non-optimized, I think some of Alex's ForwardedAtom work may help.

I'm not sure there's much left to do here.

Status: NEW → RESOLVED
Closed: 16 days ago
Duplicate of bug: 1867359
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.