bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

Store private GC things inside Values

RESOLVED FIXED in Firefox 49

Status

()

Core
JavaScript Engine
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: shu, Unassigned)

Tracking

unspecified
mozilla49
Points:
---

Firefox Tracking Flags

(firefox49 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
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.
(Reporter)

Comment 1

2 years ago
Created attachment 8746963 [details] [diff] [review]
Implement PrivateGCThingValue.

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+

Comment 7

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/fdea8d099dbd
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox49: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
(Reporter)

Comment 8

2 years ago
This relanded.
Flags: needinfo?(shu)
You need to log in before you can comment on or make changes to this bug.