1148534 - Templatize IsMarked and IsAboutToBeFinalized

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Description

[Terrence Cole [:terrence]]

Attachment: 4.6.2_templatize_ismarked-v0.diff

Nothing too complicated here. Adds a few lines since this needs explicit expansions for jsid that were not present before.

Comment 1

[Jon Coppeard (:jonco)]

Comment on attachment 8584725 [details] [diff] [review]
4.6.2_templatize_ismarked-v0.diff

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

Looks good!

::: js/src/gc/Marking.cpp
@@ +840,5 @@
> +IsMarkedInternal<Value>(Value *valuep)
> +{
> +    bool rv = true;  // Non-markable types are always live.
> +    if (valuep->isString()) {
> +        JSString *str = (JSString *)valuep->toGCThing();

Let's replace these C-style casts while we're here.

@@ +948,5 @@
> +template <typename T>
> +bool
> +IsMarkedUnbarriered(T *thingp)
> +{
> +    auto layout = reinterpret_cast<typename PtrBaseGCType<T>::type *>(thingp);

I think it should be possible to factor out this conversion.  Something like:

    template <typename T>
    PtrBaseGCType<T> *ConvertToBase(T *thing)
    {
        return reinterpret_cast ... etc
    }

Comment 2

[Steve Fink [:sfink] [:s:]]

(In reply to Jon Coppeard (:jonco) from comment #1)
> @@ +948,5 @@
> > +template <typename T>
> > +bool
> > +IsMarkedUnbarriered(T *thingp)
> > +{
> > +    auto layout = reinterpret_cast<typename PtrBaseGCType<T>::type *>(thingp);
> 
> I think it should be possible to factor out this conversion.  Something like:
> 
>     template <typename T>
>     PtrBaseGCType<T> *ConvertToBase(T *thing)
>     {
>         return reinterpret_cast ... etc
>     }

asBase, maybe? Or asBaseGCType or something.

Comment 3

[Terrence Cole [:terrence]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6a6113aac948
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/bca3bd57a71e

Comment 4

[Terrence Cole [:terrence]]

Whoops, checked this in with the wrong bug: bug 1149352.