Closed Bug 1458662 Opened 3 years ago Closed 2 years ago
Re-enable ARM64 regex JIT
46 bytes, text/x-phabricator-request
|Details | Review|
Jim says he doesn't have specific STR or tests to reproduce the crashes, other than just visiting some top sites or tests that use regexes.
A couple months back I did fix a bug in the regex engine (a nonvolatile register was not properly saved across a call into jitted code), so it may be worth retesting this just to see if it's still broken. We're running with native regexes enabled when running JS shell tests, but I don't think we're running even all the shell tests on device. Also, the bug I fixed was for a release build (high optimization levels), ISTR an unoptimized build did not trigger it. So how are we running our tests?
Jim, when you get a chance, can you try testing the ARM64 regex JIT again? Lars might have fixed the crashes you saw.
Seems to be okay now from quick testing. We should remove the lines at https://searchfox.org/mozilla-central/rev/795575287259a370d00518098472eaa5b362bfa7/mobile/android/app/mobile.js#874-876
Summary: Fix ARM64 regex JIT crashes and re-enable JIT → Re-enable ARM64 regex JIT
Marking as [geckoview:crow:p2] bug because we'd like to ship this in Crow, but we don't need it in Klar. If no one volunteers to fix this bug in the next few weeks, someone on the GeckoView team can fix it for Crow.
Dropping priority P3 so GeckoView will re-triage this bug. Is it too late to re-enable the ARM64 regex JIT before Crow ships in July?
Comment on attachment 8998000 [details] Bug 1458662 - Re-enable regex JIT on aarch64. r?jchen Jim Chen [:jchen] [:darchons] has approved the revision. https://phabricator.services.mozilla.com/D2796
Attachment #8998000 - Flags: review+
We don't need to uplift this fix to GV 62 Beta because we're not shipping an ARM64 build of Focus+GV yet.
I tested locally this in a local aarch64 build on various jsperf links I found that used lots of regexes. https://treeherder.mozilla.org/#/jobs?repo=try&revision=48e276583bc30f1b2024844ae8c0e385dc86ef71
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/d6d39130d3c9 Re-enable regex JIT on aarch64. r=jchen
(In reply to Matt Brubeck (:mbrubeck) from comment #10) > I tested locally this in a local aarch64 build on various jsperf links I > found that used lots of regexes. Did you see any speedup on the jsperf tests after enabling the ARM64 regex JIT?
(In reply to Chris Peterson [:cpeterson] from comment #13) > Did you see any speedup on the jsperf tests after enabling the ARM64 regex > JIT? Sorry, I didn't actually do before/after comparisons of the perf numbers... We could do this in Fennec nightly by flipping the pref in about:config.
You need to log in before you can comment on or make changes to this bug.