Closed Bug 1481897 Opened 2 years ago Closed 1 year ago

Remove HashPolicy template param from WeakMap, force to use MovableCellHasher<Key> instead

Categories

(Core :: JavaScript: GC, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: sfink, Assigned: sfink)

Details

Attachments

(1 file)

The possibly flawed rationale:

Weakmaps call IsMarked on their keys at various points without doing any rekeying, which means they don't handle the keys moving.
The reason why I call the above rationale possibly flawed:

The marking happens during the markIteratively phase. The only time things can move is during minor GCs and compacting GCs. In both of those cases, weakmaps are traced and marked fully (without worrying about the ephemeron properties) -- so these IsMarked calls are guaranteed not to move anything.

If that logic is correct, then the only effect of this patch is to prevent WeakMap from being used with types for which MovableCellHasher is not instantiated, and to prevent using a different HashPolicy entirely.
Attachment #8998589 - Flags: feedback?(jcoppeard)
Comment on attachment 8998589 [details] [diff] [review]
Remove HashPolicy template param from WeakMap, force to use MovableCellHasher<Key> instead

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

It looks like we removed the rekeying logic for weakmaps in bug 1223519 so I think we should just go ahead and land this.

::: js/src/gc/WeakMap.h
@@ +202,4 @@
>  {
>    public:
>      ObjectValueMap(JSContext* cx, JSObject* obj)
> +      : WeakMap<HeapPtr<JSObject*>, HeapPtr<Value>>(cx, obj)

I think you can just say WeakMap here and remove the template arguments.
Attachment #8998589 - Flags: feedback?(jcoppeard) → review+
Pushed by sfink@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9b67f745dd37
Remove HashPolicy template param from WeakMap, force to use MovableCellHasher<Key> instead, r=jonco
https://hg.mozilla.org/mozilla-central/rev/9b67f745dd37
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.