Closed Bug 686144 Opened 8 years ago Closed 8 years ago

Eliminate gc::MarkingDelay

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla9

People

(Reporter: igor, Assigned: igor)

References

Details

Attachments

(1 file)

To implement delayed marking, that we use when the GC marking requirements exceeds the capacity of the normal marking stack, we use one extra word per GC arena. The word allows to link the arena with things that caused marking delay. However, due to layout inefficiencies the real cost with 1Mb GC chunks is about 0.4% and it grows to 1.6% if we change the GC chunk size to 256K, which is one of the options I consider for the bug 671702 to decrease our heap fragmentation. 

For example, with 256K chunks without the Chunk::markingDelays arrays we can fit 63 arenas into the chunk. 63 arenas and their marking bitmaps takes 256K - 64 bytes and those 64 bytes is enough to fit ChunkInfo. But it is not enough to also fit the array of 63 marking delay words. That results in only 62 available arenas or 1.6% overhead causing noticeable regression in v8 benchmark.

To avoid that we can pack the delayed marking linkage together with ArenaHead::allocKind. The arena pointers always has ArenaShift (12 currently) low bits cleared and that is enough to fit ArenaHeader::allocKind.

The bug depends on the 686017 bug as it would make access to ArenaHeader::allocKind slightly slower and without that bug it causes visible V8 regression.
Attached patch v1Splinter Review
The patch eliminates separated Chunk::markingDelay array and merges its words with the arena header.
Attachment #559708 - Flags: review?(wmccloskey)
Summary: Eliminating gc::MarkindDelay → Eliminate gc::MarkingDelay
Comment on attachment 559708 [details] [diff] [review]
v1

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

::: js/src/jsgc.h
@@ +386,5 @@
> +     * stack.
> +     *
> +     * To minimize the ArenaHeader size we record the next delayed marking
> +     * linkage as arenaAddress() >> ArenaShift and pack it with the allocKind
> +     * field and hasDelayedMarking flag. We use 8 bites for the allocKind, not

bites->bits

@@ +392,5 @@
> +     * to access it.
> +     */
> +  public:
> +    size_t       hasDelayedMarking  : 1;
> +    size_t       nextDelayedMarking : JS_BITS_PER_WORD - 8 - 1;

Could you add a getter and a setter for nextDelayedMarking that do the bit shifts and casting?
Attachment #559708 - Flags: review?(wmccloskey) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/104fb6df714f - landed with getter and setter change
https://hg.mozilla.org/mozilla-central/rev/104fb6df714f
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
You need to log in before you can comment on or make changes to this bug.