Closed
Bug 770200
Opened 12 years ago
Closed 12 years ago
GC: Simplify use of GC lock
Categories
(Core :: JavaScript Engine, enhancement)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla16
People
(Reporter: jonco, Assigned: jonco)
References
Details
(Whiteboard: [js:t])
Attachments
(1 file, 1 obsolete file)
9.53 KB,
patch
|
Details | Diff | Splinter Review |
There's quite a bit of code scattered throughout jsgc.cpp to take/release the GC lock, but this is mostly unnecessary now that only a single thread per runtime is allowed. The locking code can be mostly factored into the functions which interact with the background thread, simplifying the code.
Assignee | ||
Comment 1•12 years ago
|
||
Possible fix. The GC lock is still used for DecommitArenas, and which I think is necessary but I'm not sure about.
Attachment #638386 -
Flags: review?(wmccloskey)
Assignee | ||
Updated•12 years ago
|
Assignee: general → jcoppeard
Comment 2•12 years ago
|
||
That seems wrong. The lock is still needed because it's used for synchronization with the helperThread.
Comment on attachment 638386 [details] [diff] [review] Possible fix Review of attachment 638386 [details] [diff] [review]: ----------------------------------------------------------------- Thanks, Jon. This looks great to me. Gregor, the lock isn't being removed here. We're just simplifying how it's acquired and released. For example, the patch acquires the lock in waitBackgroundSweepEnd, instead of in all its callers.
Attachment #638386 -
Flags: review?(wmccloskey) → review+
Updated•12 years ago
|
Whiteboard: [js:t]
Comment 4•12 years ago
|
||
Oh right. I should have looked closer.
Assignee | ||
Comment 5•12 years ago
|
||
Fix for the following failure on the try server: PROCESS-CRASH | file:///home/cltbld/talos-slave/test/build/jsreftest/tests/jsreftest.html?test=js1_5/extensions/regress-336409-2.js | application crashed (minidump found) This was because JSRuntime::onOutOfMemory takes the GC lock before calling waitBackgroundSweepOrAllocEnd().
Attachment #638386 -
Attachment is obsolete: true
Assignee | ||
Comment 6•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed582ea30746
Comment 7•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ed582ea30746
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
You need to log in
before you can comment on or make changes to this bug.
Description
•