Closed Bug 850842 Opened 11 years ago Closed 11 years ago

Various fixes for Watchpoints for generational GC

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla23

People

(Reporter: terrence, Assigned: terrence)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch v0 (obsolete) — 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+
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.
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+
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.

Attachment

General

Created:
Updated:
Size: