Closed
Bug 1470793
Opened 6 years ago
Closed 6 years ago
Stop eagerly XDR encoding scripts in the preloader cache
Categories
(Core :: XPConnect, enhancement)
Core
XPConnect
Tracking
()
RESOLVED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: kmag, Assigned: kmag)
References
(Blocks 1 open bug)
Details
(Whiteboard: [overhead:1MB])
Attachments
(1 file)
We initially started eagerly encoding cached scripts due to some flakiness we were seeing in crash reports, that we couldn't figure out the source of. That's not a big deal most of the time, but in the child process, we never wind up freeing the encoded data unless we send it back to the parent process, which we only ever do for the first content process.
In general, this should only be an issue when the startup cache is flushed, or we start loading new scripts in content processes for some other reason, but it still makes me uncomfortable (and could add up to a lot in sessions that need a cache flush).
Comment hidden (mozreview-request) |
Comment 2•6 years ago
|
||
mozreview-review |
Comment on attachment 8987402 [details]
Bug 1470793: Stop eagerly XDR encoding scripts in the preloader cache.
https://reviewboard.mozilla.org/r/252650/#review259838
::: js/xpconnect/loader/ScriptPreloader.cpp
(Diff revision 1)
> - // exist in the child cache, encode it now, before it's ever executed.
> - //
> - // Ideally, we would like to do the encoding lazily, during idle slices.
> - // There are subtle issues with encoding scripts which have already been
> - // executed, though, which makes that somewhat risky. So until that
> - // situation is improved, and thoroughly tested, we need to encode eagerly.
I guess we need to keep any eye out for new crashes then?
Attachment #8987402 -
Flags: review?(erahm) → review+
Assignee | ||
Comment 3•6 years ago
|
||
(In reply to Eric Rahm [:erahm] (please no mozreview requests) from comment #2)
> Comment on attachment 8987402 [details]
> Bug 1470793: Stop eagerly XDR encoding scripts in the preloader cache.
>
> https://reviewboard.mozilla.org/r/252650/#review259838
>
> ::: js/xpconnect/loader/ScriptPreloader.cpp
> (Diff revision 1)
> > - // exist in the child cache, encode it now, before it's ever executed.
> > - //
> > - // Ideally, we would like to do the encoding lazily, during idle slices.
> > - // There are subtle issues with encoding scripts which have already been
> > - // executed, though, which makes that somewhat risky. So until that
> > - // situation is improved, and thoroughly tested, we need to encode eagerly.
>
> I guess we need to keep any eye out for new crashes then?
Ted and nbp are already keeping track of XDR crashes. This change didn't seem to have any effect on them, though, so I'm not super worried. It was mainly just defensive until we figured things out.
Assignee | ||
Comment 4•6 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/841ed9c78bf765e7d9f40614c29853948b6680d0
Bug 1470793: Stop eagerly XDR encoding scripts in the preloader cache. r=erahm
Comment 5•6 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Comment 6•6 years ago
|
||
Seems like this won us some perf:
== Change summary for alert #14073 (as of Wed, 27 Jun 2018 16:21:09 GMT) ==
Improvements:
3% cpstartup content-process-startup linux64-qr opt e10s stylo 203.42 -> 196.92
3% cpstartup content-process-startup linux64 opt e10s stylo 189.92 -> 184.25
2% cpstartup content-process-startup linux64 pgo e10s stylo 183.17 -> 178.83
For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=14073
Comment 7•6 years ago
|
||
Plus some AWSY gains! Both these improvements are either related to this bug or bug 1471089.
== Change summary for alert #14064 (as of Wed, 27 Jun 2018 16:21:09 GMT) ==
Improvements:
16% Base Content Explicit windows7-32 pgo stylo 11,811,498.67 -> 9,922,218.67
14% Base Content Explicit windows10-64 pgo stylo 14,783,658.67 -> 12,769,280.00
12% Base Content Explicit windows10-64 opt stylo 14,468,949.33 -> 12,709,546.67
12% Base Content Explicit linux64 opt stylo 18,395,648.00 -> 16,167,936.00
For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=14064
You need to log in
before you can comment on or make changes to this bug.
Description
•