1180772 - IonMonkey: GetElem ICs should atomize incoming keys

Status: RESOLVED DUPLICATE of bug 1867359

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

Description

[Kannan Vijayan [:djvj]]

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.

Comment 1

[Guilherme Lima]

Did this got fixed in the last few weeks?

Comment 2

[Hannes Verschore [:h4writer]]

Given the description I would say this is bug 1324566 ?

Comment 3

[Jan de Mooij [:jandem]]

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.

Comment 4

[Iain Ireland [:iain]]

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.