1360523 - Make the number of reserved slots dynamic for proxy classes

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Jan de Mooij [:jandem]]

Bug 1237504 hardcoded this to 2 but it should be trivial to change that now.

It will let us shrink the size of DOM proxies because they need only 1 reserved slot instead of 2, so combined with the private slot it means we can go from an OBJECT4 allocation to OBJECT2.

I also want to use a constexpr template to static_assert the number of slots <= MAX_FIXED_SLOTS.

Comment 1

[Jan de Mooij [:jandem]]

(In reply to Jan de Mooij [:jandem] from comment #0)
> I also want to use a constexpr template to static_assert the number of slots
> <= MAX_FIXED_SLOTS.

(Slots including the private slot, of course.)

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Patch

This patch defines the number of slots explicitly for each proxy Class.

We can use a constexpr template to assert a few things about our Class flags. I can compile (and start) the browser with this, fingers crossed GCC and MSVC accept this too (I'll do a Try push of course).

I verified the static_assert works correctly: if I give a Proxy class 15 slots it compiles and with 16 slots compilation fails.

Unfortunately I had to move this code from jsfriendapi.h to js/public/Proxy.h, because we need ProxyValueArray defined there and Proxy.h includes jsfriendapi.h. Sorry about that.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8862889 [details] [diff] [review]
Patch

>+++ b/dom/base/WindowNamedPropertiesHandler.cpp
>+                  JSCLASS_HAS_RESERVED_SLOTS(1)),

Strictly speaking, this class doesn't need any reserved slots at all.  It certainly never uses them.

Maybe document that we just have this to keep the SpiderMonkey asserts happy?

r=me; thank you for doing this!

Comment 4

[Jan de Mooij [:jandem]]

Thanks for the quick review! I'll add the comment and push this later today or tomorrow.

(In reply to Jan de Mooij [:jandem] from comment #2)
> fingers crossed GCC and MSVC accept this too

They do \o/

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7844b2c91af47a2051f6d11943f59ad00bdd7654&group_state=expanded

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1d5d4015f0c1
Define number of reserved slots explicitly for each proxy js::Class. r=bz

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/1d5d4015f0c1