1131523 - Handle property accesses on object-or-primitive better

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Jan de Mooij [:jandem]]

Noticed this while investigating bug 1131099. Lodash does things like this:

  isLarge = isCommon && length >= 200,
  seen = isLarge && createCache(),

The result is that |seen| has a TypeSet that includes object + boolean and we don't use a GetProperty IC. Probably the most robust thing to do here is to inspect the Baseline ICs and (fallibly) unbox to object if we've never actually seen a primitive for this property access.

A quick workaround for this makes us 4-5x faster on those tests.

Comment 1

[Jan de Mooij [:jandem]]

John, we'll probably want to fix this in lodash too as it will affect older FF releases and probably other JS engines...

Comment 2

[John-David Dalton]

What would be the workaround?

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

(In reply to John-David Dalton from comment #2)
> What would be the workaround?

I guess assigning a null value in the !isLarge case, i.e. transforming the && into a ternary:

seen = isLarge ? createCache() : null;

Jan, can you confirm please?

Comment 4

[John-David Dalton]

Yes, this gives a massive boost – http://jsperf.com/array-object-unique/3#chart=bar

Comment 5

[Jan de Mooij [:jandem]]

Clearing the needinfo. This is on the TODO list, but keeping all these requests unnecessarily clutters my review queue and is a bit demotivating.

Comment 6

[Jan de Mooij [:jandem]]

Oops, I thought I needinfo'd myself to fix this but I didn't.

(In reply to Benjamin Bouvier [:bbouvier] from comment #3)
> Jan, can you confirm please?

Yes that looks fine and based on comment 4 it helped a lot :)

Comment 7

[Jan de Mooij [:jandem]]

The follow-up patch in bug 1169611 fixed this. Lodash (older version) improvement:

http://arewefastyet.com/#machine=29&view=single&suite=misc&subtest=bugs-1131099-lodash1
http://arewefastyet.com/#machine=29&view=single&suite=misc&subtest=bugs-1131099-lodash2