Closed
Bug 873640
Opened 11 years ago
Closed 11 years ago
Read-ahead not actually being used on omni.ja
Categories
(Core :: Networking: JAR, defect)
Core
Networking: JAR
Tracking
()
RESOLVED
FIXED
mozilla24
People
(Reporter: vladan, Assigned: glandium)
References
(Blocks 1 open bug)
Details
(Keywords: regression, Whiteboard: [Snappy:P2])
Attachments
(1 file, 1 obsolete file)
17.45 KB,
patch
|
ted
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
I noticed xperf reporting that only half of omni.ja was being read on startup on Win7, despite read-ahead having been added in bug 810151. I then stepped through the code and found that the "readaheadLength" variable is 0 for all the omni.ja's that ship with Nightly and Aurora.
Attachment #751194 -
Flags: feedback?(aklotz)
Comment 1•11 years ago
|
||
Hmm... this was working when I first checked things in. I'll verify this, but if that value is 0 then it is probably not being set correctly when omni.ja is generated in a PGO build.
Assignee | ||
Comment 2•11 years ago
|
||
Indeed, this is supposed to be filled.
Assignee | ||
Comment 4•11 years ago
|
||
This is likely a regression from bug 840094
Blocks: 840094
status-firefox22:
--- → affected
status-firefox23:
--- → affected
status-firefox24:
--- → affected
Whiteboard: [Snappy:P2] → [Snappy:P2] [regression]
Assignee | ||
Comment 5•11 years ago
|
||
The cause is actually very likely bug 852961.
Assignee | ||
Comment 6•11 years ago
|
||
Yeah, this is it, bug 852961 changed the directory under which the profile script is run, which broke the OBJDIR and JARLOG_FILE paths given to it from client.mk, because they are relative.
Updated•11 years ago
|
Keywords: regression
Whiteboard: [Snappy:P2] [regression] → [Snappy:P2]
Assignee | ||
Comment 7•11 years ago
|
||
This does a few things: - Remove profile_pageloader.*. These were added in bug 237239 and haven't been used since bug 418772 landed (profileserver.py) - Remove OBJDIR variable from the script invocation, as it was a leftover from using profile_pageloader.pl - Set JARLOG_FILE relative to the objdir, which is where the profileserver.py script is invoked from - Replace the PROFILE_GEN_SCRIPT invocation with the raw EXTRA_TEST_ARGS=10 $(MAKE) -C $(PGO_OBJDIR) pgo-profile-run ; this also means we're giving that 10 on all platforms, now, contrary to before, and I don't see any reason there should be a difference between platforms for that. And now that we have that make rule ro run the script, there's no need to use a PROFILE_GEN_SCRIPT variable to set it. - Remove PROFILE_GEN_SCRIPT definitions from mozconfigs
Attachment #751307 -
Flags: review?(ted)
Reporter | ||
Updated•11 years ago
|
Attachment #751194 -
Attachment is obsolete: true
Attachment #751194 -
Flags: feedback?(aklotz)
Comment 8•11 years ago
|
||
Comment on attachment 751307 [details] [diff] [review] Fix and cleanup profiledbuild profile script invocation Review of attachment 751307 [details] [diff] [review]: ----------------------------------------------------------------- Can you clean up the documentation after you make this change? https://developer.mozilla.org/en-US/docs/Building_with_Profile-Guided_Optimization I guess this removes the ability to have custom profiling scenarios without patching the code, but I don't think that's a big deal.
Attachment #751307 -
Flags: review?(ted) → review+
Assignee | ||
Comment 9•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d7fa77615273
Assignee | ||
Comment 10•11 years ago
|
||
Updated mdn: https://developer.mozilla.org/en-US/docs/Building_with_Profile-Guided_Optimization
Comment 11•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d7fa77615273
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Assignee | ||
Updated•11 years ago
|
Assignee | ||
Comment 12•11 years ago
|
||
Comment on attachment 751307 [details] [diff] [review] Fix and cleanup profiledbuild profile script invocation [Approval Request Comment] Bug caused by (feature/regressing bug #): regression from bug 852961 User impact if declined: Startup time regressions Testing completed (on m-c, etc.): Validated on PGO builds on m-i Risk to taking this patch (and alternatives if risky): Low String or IDL/UUID changes made by this patch: None
Attachment #751307 -
Flags: approval-mozilla-beta?
Attachment #751307 -
Flags: approval-mozilla-aurora?
Updated•11 years ago
|
Attachment #751307 -
Flags: approval-mozilla-beta?
Attachment #751307 -
Flags: approval-mozilla-beta+
Attachment #751307 -
Flags: approval-mozilla-aurora?
Attachment #751307 -
Flags: approval-mozilla-aurora+
Comment 13•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/7d718558c9d2 https://hg.mozilla.org/releases/mozilla-beta/rev/82cddf875c60
Comment 14•11 years ago
|
||
I've tried a PGO build on Linux today, but it fails: make[1]: Leaving directory `/var/tmp/moz-build-dir' rm -f /var/tmp/moz-build-dir/jarlog/en-US.log MOZ_PGO_INSTRUMENTED=1 JARLOG_FILE=jarlog/en-US.log EXTRA_TEST_ARGS=10 make -C /var/tmp/moz-build-dir pgo-profile-run make[1]: Entering directory `/var/tmp/moz-build-dir' make[1]: *** No rule to make target `pgo-profile-run'. Stop. make[1]: Leaving directory `/var/tmp/moz-build-dir' make: *** [profiledbuild] Error 2 I have the following in my .mozconfig: ac_add_options --enable-profile-guided-optimization mk_add_options PROFILE_GEN_SCRIPT=/home/markus/run-firefox.sh and run: make -f client.mk profiledbuild
You need to log in
before you can comment on or make changes to this bug.
Description
•