Closed
Bug 850842
Opened 11 years ago
Closed 11 years ago
Various fixes for Watchpoints for generational GC
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla23
People
(Reporter: terrence, Assigned: terrence)
References
Details
Attachments
(1 file, 1 obsolete file)
3.71 KB,
patch
|
billm
:
review+
|
Details | Diff | Splinter Review |
There are two fixes here: the first is to only ExposeToActiveJS on tenured things. The second is sadfaces: we cannot use a generic HashKeyRef for this table without teaching gc/Marking.h about WatchKey. I think the better solution is just to add a custom generic entry for watchpoints.
Attachment #724586 -
Flags: review?(wmccloskey)
Comment on attachment 724586 [details] [diff] [review] v0 Review of attachment 724586 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jswatchpoint.cpp @@ +43,5 @@ > + JS_ASSERT(prior.id == key_.id); > + MarkObject(trc, &key_.object, "WatchKeyRef object"); > + if (prior.object != key_.object) { > + map_->remove(prior); > + map_->put(key_, value); I'm hazy on the history of rekey. Why shouldn't it be used here?
Attachment #724586 -
Flags: review?(wmccloskey) → review+
Assignee | ||
Comment 2•11 years ago
|
||
This is the same as in HashKeyRef. |rekey| is HashTable::Enum::rekey -- it is only valid while enumerating a hash table. This usage is safe on a HashTable because we are guaranteed to have adequate space. If it attempts to resize because of tombstoning, the table will fail-safe by re-using the existing table. I suppose we could add a rekey method to HashTable, HashMap, and DebuggerWeakMap, but this approach seemed simpler since it is only the two places (iirc) that need it if we do it this way. I will look harder at this Monday and we can discuss it in more detail then.
Assignee | ||
Comment 3•11 years ago
|
||
This is a new approach. The patch is much smaller but the code is basically just different, so I'm asking for re-review. Now that we don't have to worry about pre-barriers, we can use HashKeyRef. We need to teach Mark how to handle a WatchKey, but that's much simpler than creating a custom BufferableRef.
Assignee: general → terrence
Attachment #724586 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #728507 -
Flags: review?(wmccloskey)
Attachment #728507 -
Flags: review?(wmccloskey) → review+
Assignee | ||
Comment 4•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/f41de6d5394f
Comment 5•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f41de6d5394f
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in
before you can comment on or make changes to this bug.
Description
•