Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Eliminate gc::MarkingDelay

RESOLVED FIXED in mozilla9

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Igor Bukanov, Assigned: Igor Bukanov)

Tracking

unspecified
mozilla9
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
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.
(Assignee)

Comment 1

6 years ago
Created attachment 559708 [details] [diff] [review]
v1

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+
(Assignee)

Comment 3

6 years ago
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
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
You need to log in before you can comment on or make changes to this bug.