1339411 - Rewrite and optimize object allocation paths

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jan de Mooij [:jandem]]

Attachment: Patch

Right now all of our VM object allocations go through NewObject* and then JSObject::create.

Problem is that the different object types (for example unboxed objects and proxies) have very little in common, so there are a ton of branches to handle special cases that are only relevant for some objects, and this makes the code pretty slow and hard to reason about.

The attached patch splits JSObject::create in:

* NativeObject::create
* ProxyObject::create
* TypedObject::create
* UnboxedObject::createInternal (to distinguish from Unboxed*Object::create)

Furthermore, proxies and unboxed objects no longer go through the NewObject* machinery at all, it's simpler and faster to inline the few bits we actually needed from that. (This also means when we allocate an unboxed object, we no longer look up an initial shape that we don't use.)

This patch already is a measurable win on a wrapper allocation micro-benchmark, but the next step is optimizing ProxyObject::create more (Google Docs creates a ton of wrappers) by caching the shape and group. Doing that will be much simpler on top of this patch.

Finally, this should also make it easier to experiment with different proxy layouts in the future, bug 1237504.

Comment 1

[Brian Hackett [Laid off!]]

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

Review of attachment 8837126 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/builtin/TypedObject.cpp
@@ +2400,5 @@
> +    // GetReservedOrProxyPrivateSlot.
> +    MOZ_ASSERT(JSCLASS_RESERVED_SLOTS(clasp) == 0);
> +    MOZ_ASSERT(!clasp->hasPrivate());
> +    MOZ_ASSERT(shape->numFixedSlots() == 0);
> +    MOZ_ASSERT(shape->slotSpan() == 0);

I like the idea and speed and cleanliness improvements here, but a bunch of these assertions are now being duplicated in multiple places with subtle variations that tie into engine wide invariants we expect to hold, like the thing for GetReservedOrProxyPrivateSlot.

Can you expand debugCheckFinalizeFlags into something like debugCheckNewObject, which takes the group, shape, and other bits and asserts all the properties we expect to hold for all objects in the engine?

Comment 2

[Jan de Mooij [:jandem]]

Sure, that's a fair suggestion. I was thinking not all of the asserts would be relevant for each of the new allocation sites, but yeah maybe better to have a consistent set of checks. I'll update the patch.

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Patch v2

Comment 4

[Brian Hackett [Laid off!]]

Comment on attachment 8837183 [details] [diff] [review]
Patch v2

Review of attachment 8837183 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/12667d6bc208
Rewrite and optimize object allocation paths. r=bhackett

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/12667d6bc208