Closed
Bug 1169692
Opened 9 years ago
Closed 9 years ago
Make WeakMapTracer use virtual dispatch instead of a function pointer
Categories
(Core :: JavaScript: GC, defect)
Core
JavaScript: GC
Tracking
()
RESOLVED
FIXED
mozilla41
Tracking | Status | |
---|---|---|
firefox41 | --- | fixed |
People
(Reporter: terrence, Assigned: terrence)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
5.39 KB,
patch
|
sfink
:
review+
|
Details | Diff | Splinter Review |
5.89 KB,
patch
|
mccr8
:
review+
|
Details | Diff | Splinter Review |
I based this under bug 1169086 in hopes it would make some of the hazards disappear. No such luck. It doesn't introduce any hazards on its own though, so we should be able to land this easily at least.
Attachment #8612959 -
Flags: review?(sphink)
Assignee | ||
Comment 1•9 years ago
|
||
And the gecko bits.
Attachment #8612961 -
Flags: review?(continuation)
Comment 2•9 years ago
|
||
Comment on attachment 8612959 [details] [diff] [review] sm_virtualize_WeakMapTracer-v0.diff Review of attachment 8612959 [details] [diff] [review]: ----------------------------------------------------------------- I am a big fan of function pointer -> vtable changes.
Attachment #8612959 -
Flags: review?(sphink) → review+
Comment 3•9 years ago
|
||
Comment on attachment 8612961 [details] [diff] [review] gecko_virtualize_WeakMapTracer-v0.diff Review of attachment 8612961 [details] [diff] [review]: ----------------------------------------------------------------- ::: xpcom/base/CycleCollectedJSRuntime.cpp @@ +246,5 @@ > } > > private: > > + void trace(JSObject* aMap, JS::GCCellPtr aKey, JS::GCCellPtr aValue) override Doesn't trace have to be public or something?
Attachment #8612961 -
Flags: review?(continuation) → review+
Assignee | ||
Comment 4•9 years ago
|
||
I would have thought so to. I forgot to change it after I built successfully. And it seems a full Try run is green as well, so I guess not? https://treeherder.mozilla.org/#/jobs?repo=try&revision=0b0c4ea97df6
Comment 5•9 years ago
|
||
Well, please make it public to make it less confusing at least. ;)
Assignee | ||
Comment 6•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0b0c4ea97df6
Comment 8•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7613d2aadc7e
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in
before you can comment on or make changes to this bug.
Description
•