Closed Bug 1268805 Opened 4 years ago Closed 4 years ago

Store private GC things inside Values

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: shu, Unassigned)

Details

Attachments

(1 file)

Often in Classes with reserved slots I want to store a GC thing that's not an Object, String, or Symbol. Having a PrivateGCThingValue that's automatically handled by GC would be very convenient.
I noticed no performance regressions on octane.
Attachment #8746963 - Flags: review?(terrence)
Comment on attachment 8746963 [details] [diff] [review]
Implement PrivateGCThingValue.

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

Looks great!

::: js/public/HeapAPI.h
@@ +392,5 @@
>      return js::gc::detail::CellIsMarkedGray(thing.asCell());
>  }
>  
> +extern JS_PUBLIC_API(JS::TraceKind)
> +GCThingTraceKind(void* thing);

Can this be js::gc::Cell* here? If not no worries.

::: js/src/gc/Marking.h
@@ +438,5 @@
> +                             !mozilla::IsBaseOf<JS::Symbol, T>::value, T>
> +{
> +    static_assert(!mozilla::IsSame<Cell, T>::value && !mozilla::IsSame<TenuredCell, T>::value,
> +                  "T must not be Cell or TenuredCell");
> +};

Nice!

::: js/src/jsapi-tests/testPrivateGCThingValue.cpp
@@ +13,5 @@
> +class TestTracer : public JS::CallbackTracer
> +{
> +    void onChild(const JS::GCCellPtr& thing) override {
> +        printf("*thingp      = %p\n", thing.asCell());
> +        printf("kind         = %d\n", static_cast<int>(thing.kind()));

Comment or remove the prints before checking in.
Attachment #8746963 - Flags: review?(terrence) → review+
https://hg.mozilla.org/mozilla-central/rev/fdea8d099dbd
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
This relanded.
Flags: needinfo?(shu)
You need to log in before you can comment on or make changes to this bug.