1236523 - Some Shape::search optimizations

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jan de Mooij [:jandem]]

Even though the JITs can use ICs in most cases, Shape::search can be very hot on some workloads and it's easy to squeeze some more performance out of it.

I'll attaching patches to:

(1) Make the 'bool adding' argument to ShapeTable::search a template parameter. This lets the compiler get rid of a bunch of unnecessary branches in the common (not adding) case.

(2) Cache the result of Shape::isBigEnoughForAShapeTable() on the Shape. This function is called a lot of times, especially when isBigEnoughForAShapeTable == false, and doing a shape walk each time is not necessary.

Together these patches are a 15-20% win on some worst-case micro-benchmarks.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Part 1 - Remove NON_NATIVE flag

Nowadays the NON_NATIVE Shape flag is only used for one assert. We can just use clasp->isNative() instead.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Part 2 - Cache isBigEnoughForAShapeTable

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Part 3 - Templatize Shape::search and ShapeTable::search

Comment 4

[Brian Hackett [Laid off!]]

Comment on attachment 8703598 [details] [diff] [review]
Part 1 - Remove NON_NATIVE flag

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

::: js/src/vm/Shape.h
@@ +717,2 @@
>  
>          UNUSED_BITS     = 0x3C

This should be removed I think; it was wrong before and after this change it still looks wrong.

Comment 5

[Brian Hackett [Laid off!]]

Comment on attachment 8703602 [details] [diff] [review]
Part 3 - Templatize Shape::search and ShapeTable::search

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

::: js/src/vm/Shape.h
@@ +120,5 @@
>  /* Limit on the number of slotful properties in an object. */
>  static const uint32_t SHAPE_INVALID_SLOT = JS_BIT(24) - 1;
>  static const uint32_t SHAPE_MAXIMUM_SLOT = JS_BIT(24) - 2;
>  
> +enum class AddingBool { Adding = true, NotAdding = false };

AddingBool is a bit awkward.  Maybe MaybeAdding?

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fe453285a593
https://hg.mozilla.org/integration/mozilla-inbound/rev/f8817ab2f745
https://hg.mozilla.org/integration/mozilla-inbound/rev/cb2aea4df005

Comment 7

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

https://hg.mozilla.org/mozilla-central/rev/fe453285a593
https://hg.mozilla.org/mozilla-central/rev/f8817ab2f745
https://hg.mozilla.org/mozilla-central/rev/cb2aea4df005