Closed Bug 1378736 Opened 2 years ago Closed 2 years ago

Consider not nulling out RegExpObject -> RegExpShared pointer on GC

Categories

(Core :: JavaScript Engine, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: jandem, Assigned: jandem)

References

Details

Attachments

(2 files)

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.
Depends on: 1378740
This is pretty straight-forward. sharedRef now returns a PreBarriered instead of ReadBarriered RegExpShared*.
Attachment #8884246 - Flags: review?(jcoppeard)
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)
Attachment #8884246 - Flags: review?(jcoppeard) → review+
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
https://hg.mozilla.org/mozilla-central/rev/6561b74cec5c
https://hg.mozilla.org/mozilla-central/rev/8a1f2f23edb0
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.