Open Bug 1453142 Opened 3 years ago Updated 3 years ago

Testcase where we're not optimizing DOM things in top-level script with global scope assignment.

Categories

(Core :: JavaScript Engine, enhancement, P3)

Product:
Core
Component:
JavaScript Engine
Type:
enhancement

Tracking

()

People

(Reporter: bzbarsky, Unassigned)

References

Details

(Keywords: perf, Whiteboard: [qf:p3]) 

It's really simple:

  <!DOCTYPE html>
  <script>
    var start = new Date();
    var count = 10000000;
    var nodes;
    for (var i = 0; i < count; ++i) {
      nodes = document.getElementsByClassName("foo");
    }
    var stop = new Date();
    alert((stop - start)/count * 1e6);    
</script>

The i++ and nodes assignments should not alias DOM things, so I would expect us to be able to loop-hoist the document.getElementsByClassName() bit and the alert should alert a number in the 1-1.5 (ns/iteration) range.  I see numbers closer to 8.

Jan, do you have time to take a look, or know who might?
Flags: needinfo?(jdemooij)
It looks like we do optimize the getElementsByClassName call, but we're still doing a getprop in the loop body (see below). This must be the document.getElementsByClassName - we're then guarding that returns the function we expect...

* LInterruptCheck

// i < count
* LoadSlotT
* CompareAndBranch:lt

// Some getprop 
* GetPropertyCacheV
* Unbox:Object
* TypeBarrierO

// nodes = ...
* PostWriteBarrierO
* StoreSlotT

// ++i
* AddI
* StoreSlotT

* Goto
I think the IC code ends up in GetPropIRGenerator::tryAttachDOMProxyUnshadowed. We would need the equivalent of these does-not-shadow checks in IonBuilder, right? Do you know if this used to be inlined?

Bug 1324561 would let us inline this in IonBuilder based on what the Baseline IC did.
> This must be the document.getElementsByClassName

Oh, hmm...

So one interesting thing is that if I put all this inside a function (which returns the list, so we can't dead-code-eliminate all this stuff) things do get optimized.

I guess that means the global slot assignment is aliasing the getprop, basically?  I'm not actually quite sure now how we manage to optimize this in the inside-a-function case.
Depends on: 1324561
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #3)
> I guess that means the global slot assignment is aliasing the getprop,
> basically?  I'm not actually quite sure now how we manage to optimize this
> in the inside-a-function case.

I mentioned this on IRC yesterday, but for posterity:

<jandem> bz: we use an idempotent IC for this, so in the function case we can loop hoist it I expect. Idempotent ICs only support a few operations, including unshadowed dom proxies
Flags: needinfo?(jdemooij)
Keywords: perf
Priority: -- → P3
Summary: Testcase where we're not optimizing DOM things as I would expect us to → Testcase where we're not optimizing DOM things in top-level script with global scope assignment.
Whiteboard: [qf]
Whiteboard: [qf] → [qf:p3:f64]
Whiteboard: [qf:p3:f64] → [qf:p3]
You need to log in before you can comment on or make changes to this bug.