Closed Bug 1243888 Opened 4 years ago Closed 4 years ago

Derive RootKind automatically from TraceKind

Categories

(Core :: JavaScript: GC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: terrence, Assigned: terrence)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

RootKind is this weird wart that is almost but not exactly like TraceKind. Now that we have TraceKind well established in the public API, we can derive RootKind automatically from TraceKind.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=479ddbf4cf6f
Attachment #8713351 - Flags: review?(sphink)
Comment on attachment 8713351 [details] [diff] [review]
2.1_derive_RootKind_from_TraceKind-v0.diff

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

I am very happy to see this cruft go.

::: js/public/TraceKind.h
@@ +89,5 @@
>      };
>  JS_FOR_EACH_TRACEKIND(JS_EXPAND_DEF);
>  #undef JS_EXPAND_DEF
>  
> +// RootKind is closely related to TraceKind. Whereas TraceKind's indicies are

indices or indexes (both are equally valid), not indicies

@@ +105,5 @@
> +    Shape,
> +    String,
> +    Symbol,
> +
> +    // These tagged-pointers are special-cased for performance.

why the hyphen on tagged pointers?

@@ +115,5 @@
> +
> +    Limit
> +};
> +
> +// Most RootKind correspond directly to a trace kind.

"....Exceptions are specialized below."

@@ +126,5 @@
> +#undef JS_EXPAND_DEF
> +
> +// Specify the RootKind for all types. Value and jsid map to special cases;
> +// pointer types we can derive directly from the TraceKind; everything else
> +// should go in the Traceable list and use GCPolicy<T>::trace for tracing.

Very nice comment summing things up.

::: js/src/gc/RootMarking.cpp
@@ +58,4 @@
>  {
> +#define MARK_ROOTS(name, type, _) \
> +    MarkExactStackRootList<type*>(trc, stackRoots_[JS::RootKind::name], "exact-object");
> +JS_FOR_EACH_TRACEKIND(MARK_ROOTS)

This calls everything "exact-object". Did you mean "exact-" ## name? (Hm... should these be "root-foo" today?)

::: js/src/jspubtd.h
@@ -275,5 @@
> -    THING_ROOT_ID,
> -    THING_ROOT_VALUE,
> -    THING_ROOT_TRACEABLE,
> -    THING_ROOT_LIMIT
> -};

\o/ Death to THING_ROOT_STOP_SHOUTING_AT_ME!
Attachment #8713351 - Flags: review?(sphink) → review+
Blocks: 1244358
https://hg.mozilla.org/mozilla-central/rev/5eaf5e034e6d
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.