Closed Bug 1088307 Opened 5 years ago Closed 5 years ago

Pull out WeakMap unbarriering to a single location

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: sfink, Assigned: sfink)

Details

Attachments

(1 file, 1 obsolete file)

I recently reviewed a patch that cut & paste this gunk, and it was revolting.
So I decided to make it even worse. Should we do something like this? Or should this be made part of putGeneric? I think there are other places that use a DebuggerWeakMap or something that this won't handle.
Attachment #8510607 - Flags: feedback?(terrence)
Comment on attachment 8510607 [details] [diff] [review]
Pull out WeakMap unbarriering to a single location

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

Yeah, this is definitely worth taking, even if it doesn't cover all cases, just to avoid the one dup.

::: js/src/jsweakmap.h
@@ +281,5 @@
>  };
>  
> +template <class Key, class Value>
> +static inline gc::HashKeyRef<HashMap<Key, Value, DefaultHasher<Key>, RuntimeAllocPolicy>, Key>
> +unbarrieredRef(WeakMap< PreBarriered<Key>, RelocatablePtr<Value> > *map, Key key) {

{ on newline.

@@ +292,5 @@
> +
> +    typedef typename WeakMap< PreBarriered<Key>, RelocatablePtr<Value> >::Base BaseMap;
> +    auto baseMap = static_cast<BaseMap*>(map);
> +    typedef HashMap<Key, Value, DefaultHasher<Key>, RuntimeAllocPolicy> UnbarrieredMap;
> +    return gc::HashKeyRef<HashMap<Key, Value, DefaultHasher<Key>, RuntimeAllocPolicy>, Key>(reinterpret_cast<UnbarrieredMap*>(baseMap), key);

Please add a second typedef for the Ref to make this line shorter.
Attachment #8510607 - Flags: feedback?(terrence) → feedback+
I implemented this because I was reviewing code where bhackett was adding a third instance. I don't think that patch has landed yet though.
Attachment #8512240 - Flags: review?(terrence)
Attachment #8510607 - Attachment is obsolete: true
Comment on attachment 8512240 [details] [diff] [review]
Pull out WeakMap unbarriering to a single location

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

::: js/src/jsweakmap.h
@@ +285,5 @@
> + * Localize the templatized casting craziness here.
> + */
> +template <class Key, class Value>
> +static inline gc::HashKeyRef<HashMap<Key, Value, DefaultHasher<Key>, RuntimeAllocPolicy>, Key>
> +UnbarrieredRef(WeakMap< PreBarriered<Key>, RelocatablePtr<Value> > *map, Key key)

No need to separate >> anymore, so remove the spaces before and after the ><.
Attachment #8512240 - Flags: review?(terrence) → review+
https://hg.mozilla.org/mozilla-central/rev/dab206576a0c
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.