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.

Component: DOM → JavaScript Engine: JIT

Flags: needinfo?(bzbarsky)

Summary: accessing expando properties on document is very slow → setting an already-existing expando property on document is very slow

(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.

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).

> 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).

(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).