684595 - Don't visit WeakMap keys from the WeakMap in markEntry

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Andrew McCreight [:mccr8]]

During non-GC marking, we visit keys and values in a WeakMap.  Visiting values is nice because it means that the reachability relation implied by JS_TraceChildren is a strict subset of the real reachability relation, without the overhead of actually marking keys to get the true reachability relation.  However, I can't really see any justification for visiting keys, as there's never any path from a WeakMap to the key that it contains, so let's get rid of that.

See Bug 681104 for a list of non-GCmarking clients that use this.  It looks to me like they should all be okay with this change.

Comment 1

[Andrew McCreight [:mccr8]]

Attachment: don't visit keys in markEntry

Comment 2

[Andrew McCreight [:mccr8]]

Try run of this came back looking okay.

Comment 3

[Mozilla RelEng Bot]

Try run for 59444eacb69a is complete.
Detailed breakdown of the results available here:
    http://tbpl.allizom.org/?tree=Try&usebuildbot=1&rev=59444eacb69a
Results (out of 152 total builds):
    exception: 1
    success: 143
    warnings: 7
    failure: 1
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-59444eacb69a

Comment 4

[Jason Orendorff [:jorendorff]]

Comment on attachment 558160 [details] [diff] [review]
don't visit keys in markEntry

OK, looks good. While you're in here, would you please fix the comment about markEntryIfLive, just above the part you modified? It says:

//        A typical definition of markIteratively would be:

but the method name has changed; it's not markIteratively anymore.

Comment 5

[Andrew McCreight [:mccr8]]

Will do, thanks!

Comment 6

[Andrew McCreight [:mccr8]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/1c432b6fdbdb

Comment 7

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/1c432b6fdbdb

Comment 8

[Terrence Cole [:terrence]]

For the post-verifier, we really need the keys to be visited as well.

Bug 772229

Comment 9

[Andrew McCreight [:mccr8]]

In the interest of completeness, I should say that I have no idea why I decided we should not visit keys.  But I was quite convinced of it at the time...