Closed
Bug 1378736
Opened 5 years ago
Closed 5 years ago
Consider not nulling out RegExpObject -> RegExpShared pointer on GC
Categories
(Core :: JavaScript Engine, enhancement)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: jandem, Assigned: jandem)
References
Details
Attachments
(2 files)
2.47 KB,
patch
|
jonco
:
review+
|
Details | Diff | Splinter Review |
23.15 KB,
patch
|
jonco
:
review+
|
Details | Diff | Splinter Review |
This is a weak pointer and we can currently null it on GC (allowing us to collect the RegExpShared). However, RegExpShared is not that big (we can discard regex JIT code independently) and getting rid of this weak pointer and read barrier might help regexp perf and it would simplify bug 1115355 a lot. Let's do some Try pushes and compare AWSY numbers.
Assignee | ||
Comment 1•5 years ago
|
||
This is pretty straight-forward. sharedRef now returns a PreBarriered instead of ReadBarriered RegExpShared*.
Attachment #8884246 -
Flags: review?(jcoppeard)
Assignee | ||
Comment 2•5 years ago
|
||
RegExpShared can hold onto malloc'd data tables that are used by irregexp JIT code. Now that RegExpShared might be kept alive longer, it would be nice to discard these tables whenever we discard regexp JIT code. This is pretty straight-forward, except there's one problem with the current approach. In theory at least we can have the following scenario: (1) Add the table to RegExpShared while we're compiling the regexp code. (2) Then potentially GC and collect these tables. (3) Then finish regexp compilation that relies on the now discarded tables. So I refactored things a bit to add these tables to a Vector and then add them to the RegExpShared at the end of the compilation. I also changed this code to use UniquePtr, so we no longer have to free each table explicitly.
Attachment #8884249 -
Flags: review?(jcoppeard)
Updated•5 years ago
|
Attachment #8884246 -
Flags: review?(jcoppeard) → review+
Comment 3•5 years ago
|
||
Comment on attachment 8884249 [details] [diff] [review] Part 2 - Discard data tables Review of attachment 8884249 [details] [diff] [review]: ----------------------------------------------------------------- This looks good. Use of UniquePtr is nice too.
Attachment #8884249 -
Flags: review?(jcoppeard) → review+
Pushed by jandemooij@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/6561b74cec5c part 1 - Don't null out RegExpObject -> RegExpShared pointer on GC. r=jonco https://hg.mozilla.org/integration/mozilla-inbound/rev/8a1f2f23edb0 part 2 - Discard RegExpShared data tables when discarding regexp JIT code. r=jonco
Comment 5•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6561b74cec5c https://hg.mozilla.org/mozilla-central/rev/8a1f2f23edb0
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in
before you can comment on or make changes to this bug.
Description
•