Closed Bug 1236523 Opened 7 years ago Closed 6 years ago

Some Shape::search optimizations


(Core :: JavaScript Engine, defect)

Not set



Tracking Status
firefox46 --- fixed


(Reporter: jandem, Assigned: jandem)



(3 files)

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.
Nowadays the NON_NATIVE Shape flag is only used for one assert. We can just use clasp->isNative() instead.
Attachment #8703598 - Flags: review?(bhackett1024)
Attachment #8703599 - Flags: review?(bhackett1024)
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.
Attachment #8703598 - Flags: review?(bhackett1024) → review+
Attachment #8703599 - Flags: review?(bhackett1024) → review+
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?
Attachment #8703602 - Flags: review?(bhackett1024) → review+
You need to log in before you can comment on or make changes to this bug.