Closed
Bug 1236523
Opened 5 years ago
Closed 5 years ago
Some Shape::search optimizations
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla46
Tracking | Status | |
---|---|---|
firefox46 | --- | fixed |
People
(Reporter: jandem, Assigned: jandem)
Details
Attachments
(3 files)
3.63 KB,
patch
|
bhackett1024
:
review+
|
Details | Diff | Splinter Review |
2.67 KB,
patch
|
bhackett1024
:
review+
|
Details | Diff | Splinter Review |
17.04 KB,
patch
|
bhackett1024
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•5 years ago
|
||
Nowadays the NON_NATIVE Shape flag is only used for one assert. We can just use clasp->isNative() instead.
Attachment #8703598 -
Flags: review?(bhackett1024)
Assignee | ||
Comment 2•5 years ago
|
||
Attachment #8703599 -
Flags: review?(bhackett1024)
Assignee | ||
Comment 3•5 years ago
|
||
Attachment #8703602 -
Flags: review?(bhackett1024)
Comment 4•5 years ago
|
||
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+
Updated•5 years ago
|
Attachment #8703599 -
Flags: review?(bhackett1024) → review+
Comment 5•5 years ago
|
||
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+
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•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/fe453285a593 https://hg.mozilla.org/mozilla-central/rev/f8817ab2f745 https://hg.mozilla.org/mozilla-central/rev/cb2aea4df005
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
status-firefox46:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in
before you can comment on or make changes to this bug.
Description
•