1105621 - Slower than Safari on DOM binding testcase

Status: RESOLVED WORKSFORME

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

Description

[Boris Zbarsky [:bzbarsky]]

See attachment 588050 [details].  We're about 2x slower than Safari there.

Comment 1

[Boris Zbarsky [:bzbarsky]]

So I'm seeing a few issues here:

1)  We're not taking the Ion fast path for .length on the nodelist.  This costs us at least 10% of the time.

2)  The indexed get is a vmcall via js::GetElement and then proxy machinery.  This overhead costs us at least 15% of the time.

#2 is basically bug 917509.  Looking into #1.

Comment 2

[Boris Zbarsky [:bzbarsky]]

#1 is because the baseline stub here is a GetProp_CallDOMProxyNative, which commonGetPropFunction knows nothing about, so we can't do the optimization in getPropTryCommonGetter.

Correctly handling this case would probably require either a guard or a TI freeze on the proxy's expando object to make sure it's not shadowing "length".  Or something.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Eric, you were asking about DOM/JIT perf issues, right?  Here's one (or perhaps two if you count bug 917509).

Comment 4

[Eric Faust [:efaust]]

Adding Naveed for radar.

I'm not sure when I'll have an immediate chance to looks at this, but it's good to know. Leaving the ni? so it doesn't slip all the way.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Talking about this on IRC suggests that a TI freeze on "no one has added an expando named length to a DOM list (of this type?)" should in fact be possible.  I'd much prefer that to a shape guard, obviously.  ;)

Comment 6

[Mayank Bansal]

Nightly : 106,51
Chrome: 160,90

Should we close this bug as FIXED?

Comment 7

[Matthew Gaudet (he/him) [:mgaudet]]

100% (tho, I'll use WORKSFORME rather as FIXED causes release managers to peek at this)