Closed Bug 1215337 Opened 5 years ago Closed 5 years ago

Cache slotSpan()

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: sfink, Assigned: sfink)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

While single-stepping in gdb, I found a place where slotSpan() gets called for every iteration through a loop.
Attached patch Cache spotSpan()Splinter Review
Yes, I should time this and things. But I don't want to spend the time, and the resulting code is at least as easy to follow.
Attachment #8674579 - Flags: review?(terrence)
Summary: Cache spotSpan() → Cache slotSpan()
Comment on attachment 8674579 [details] [diff] [review]
Cache spotSpan()

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

::: js/src/jsobj.cpp
@@ +3703,5 @@
>          {
>              GetObjectSlotNameFunctor func(nobj);
>              JS::AutoTracingDetails ctx(trc, func);
>              JS::AutoTracingIndex index(trc);
> +            uint32_t nslots = nobj->slotSpan(); // cache

Might as well use |const uint32_t|, since that's kinda what you're aiming to show the compiler.

Also, "cache" isn't really that clear. How about a full comment like: // Since tracing can mutate the target, the compiler will not generally be able to prove that slotSpan is constant across the loop.
Attachment #8674579 - Flags: review?(terrence) → review+
https://hg.mozilla.org/mozilla-central/rev/0fbd23348147
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.