If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

GC: Unbitrot the post-barrier verifier

RESOLVED FIXED in mozilla21

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: terrence, Unassigned)

Tracking

Trunk
mozilla21
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
Created attachment 704075 [details] [diff] [review]
v0

This was surprisingly not bad, considering how long it has been. There are still a few test failures, but --disable-e4x is polluting the set so I don't really know how many of them are valid yet.

I've also added a --ggc flag to the interpreter, so that I can test performance changes quickly. It's a bit gross, so if there is a better way to do it, please let me know.
Attachment #704075 - Flags: review?(wmccloskey)
Comment on attachment 704075 [details] [diff] [review]
v0

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

::: js/src/gc/Barrier-inl.h
@@ -382,2 @@
>    public:
>      SlotRangeRef(JSObject *obj, uint32_t start, uint32_t end)

This is only used for dense arrays, so I think you should rename it to DenseRangeRef or something.

@@ +389,5 @@
>      bool match(void *location) {
> +        uint32_t len = owner->getDenseInitializedLength();
> +        for (uint32_t i = Min(start, len); i < Min(end, len); ++i) {
> +            if (&owner->getDenseElement(i) == location)
> +                return true;

...and maybe optimize this check, since dense array elements are always contiguous.

@@ +395,5 @@
>          uint32_t span = owner->slotSpan();
>          for (uint32_t i = Min(start, span); i < Min(end, span); ++i) {
>              if (owner->getSlotAddress(i) == location)
>                  return true;
>          }

...and then get rid of the slots case.

@@ +412,1 @@
>          MarkObjectSlots(trc, owner, clampedStart, Min(end, span) - clampedStart);

...and this one too.

::: js/src/gc/StoreBuffer.h
@@ +9,5 @@
>  #ifndef jsgc_storebuffer_h___
>  #define jsgc_storebuffer_h___
>  
> +#ifndef JSGC_USE_EXACT_ROOTING
> +# error "Exact rooting must be enable to enable Generational GC."

How about just "Generation GC requires exact rooting."?

@@ +12,5 @@
> +#ifndef JSGC_USE_EXACT_ROOTING
> +# error "Exact rooting must be enable to enable Generational GC."
> +#endif
> +#ifdef JSGC_HAS_XML_SUPPORT
> +# error "E4X must be disabled to enable Generational GC."

No reason to capitalize generational.

::: js/src/jsapi.h
@@ +4017,5 @@
>  
>      /* Lower limit after which we limit the heap growth. */
> +    JSGC_ALLOCATION_THRESHOLD = 20,
> +
> +    /* Enable the Generational GC. */

No reason to capitalize generational.
Attachment #704075 - Flags: review?(wmccloskey) → review+
(Reporter)

Comment 2

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/84edc4c47182
https://hg.mozilla.org/mozilla-central/rev/84edc4c47182
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.