This was found by Neil Gershenfeld at MIT. Accessing properties of document is extremely slow in Firefox in comparison to Chrome. The attached test case takes 6576 ms in Firefox and 25 ms in Chrome.

if the expando is set to an Element, I see 22ms on Nightly and 30ms in Chrome. This is all about the proxy setup we have for Document.

Bz, this is probably a dupe but not sure which. Bug 965992?

Yes, exactly. I'd actually forgotten about that one... That said, this testcase has two things that are slow-ish. There's the get, which is bug 965992. And there's the set. In practice, for this testcase, the Proxy::get is about 16% of the time and the Proxy::set is 82% of the time. That's because the set ends up in DOMProxyHandler::set which does getOwnPropDescriptor (21% of the time, mostly the JS_GetPropertyDescriptorById on the expando object) and then the defineProperty bits which do the actual set, mostly under js_DefineOwnProperty. So let's do the get in bug 965992 and then the set in this bug. That will still leave _adding_ expando props to a document being somewhat slow, but I'm not sure how much we care about that.

> but I'm not sure how much we care about that. Because if I change the testcase to do: document[n] = n; instead of document.x++, then I see us take about 6s still, while

Jan, as long as you're adding ICs, thoughts on this one?

(In reply to Boris Zbarsky [:bz] (still a bit busy) from comment #5) > Jan, as long as you're adding ICs, thoughts on this one? I'm currently working on converting our SETPROP/SETELEM stubs to CacheIR. Once that's done (few weeks from now probably) it will be easy to optimize this. It's going to be very similar to what we did for *getting* expando properties in bug 965992 part 2. Keeping the needinfo until then.

We need telemetry to determine the impact of this. Set to investigate for now.

I fixed the main blocker this week. I'll have a patch for this next week or so. Adding Telemetry for this will likely take more time than writing the patch.

This optimizes both setting expando properties and expando setter calls. It improves the attached testcase from 3711 ms to 94 ms, about 40x faster. I get 25 ms in Chrome but I wonder if maybe |document| is not a proxy in Blink? If I use a NodeList instead we are faster than them with this patch. Anyway, this is a big improvement and we can improve this more by compiling CacheIR to MIR (bug 1324561).

Comment on attachment 8849537 [details] [diff] [review] Patch Review of attachment 8849537 [details] [diff] [review]: ----------------------------------------------------------------- Very straightforward patch for such a huge perf win.

> but I wonder if maybe |document| is not a proxy in Blink? Correct, it's not. I don't understand why the changes to dom/bindings/test/test_proxy_expandos.html are needed in the patch... Apart from that, this looks awesome. ;)

(In reply to Boris Zbarsky [:bz] (still a bit busy) (if a patch has no decent message, automatic r-) from comment #11) > I don't understand why the changes to > dom/bindings/test/test_proxy_expandos.html are needed in the patch... I added this test for bug 965992. It only tests *getting* expando properties (data properties + getters). I changed it to also test the new cases we're optimizing here (setting properties + setters).

Hmm. I guess the |obj.foo = "bar";| bit in the test is just dead code, then? Or is it needed to ensure that in the loop we are always setting an existing prop?

(In reply to Boris Zbarsky [:bz] (still a bit busy) (if a patch has no decent message, automatic r-) from comment #13) > Or is it needed to ensure that in the loop we are always setting an > existing prop? Yeah I kept it for that reason, but we could remove it and the loop will still test the same things after the first iteration (a single failure-to-attach is not going to disable the IC + the first iteration likely runs in the interpreter anyway).

Pushed by jandemooij@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/485abf63b62b Optimize sets of expando properties and expando setter calls on DOM proxies. r=evilpie