Closed
Bug 1096280
Opened 10 years ago
Closed 10 years ago
Add more benchmarks to the PGO profile
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
WONTFIX
mozilla36
People
(Reporter: jandem, Assigned: jandem, NeedInfo)
References
Details
Attachments
(2 files)
8.24 KB,
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
14.25 KB,
patch
|
h4writer
:
review+
|
Details | Diff | Splinter Review |
With the VS 2013 update a few weeks ago, PGO was enabled again for js/src. In bug 1030706 we found that adding Octane and Kraken to the profile was a win. Octane exercises a lot more (interesting) code paths than Sunspider. Octane-zlib for instance tests asm.js compilation. I'm not sure about Kraken because most tests don't spend that much time in the VM, maybe I'll add some of them or modify them a bit to run faster. Boris, do you have any suggestions for interesting DOM code paths to test? It probably can't hurt to throw in some jQuery, getElementsByTagName etc stuff.
Flags: needinfo?(bzbarsky)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•10 years ago
|
||
This patch moves the sunspider files from js-input/ to js-input/sunspider/. I also changed the timeout from 3000 to 1000 to save some time.
Attachment #8519913 -
Flags: review?(ted)
Assignee | ||
Comment 2•10 years ago
|
||
Add Octane to the profile. I made a small change to base.js so that it behaves more like the shell version: run the whole test without setTimeout in the middle. The PGO index.html also uses setTimeout and we don't want them to interfere.
Attachment #8519983 -
Flags: review?(hv1989)
Comment 3•10 years ago
|
||
Some jQuery stuff would be good, yes. Perhaps something with getting/setting innerHTML and then flushing layout? In general, the browser UI exercises a bunch of DOM stuff... Olli, any suggestions here?
Flags: needinfo?(bzbarsky) → needinfo?(bugs)
Comment 4•10 years ago
|
||
yeah, innerHTML + layout flushing certainly (we need to do whatever we can to make layout flushes faster). I wonder...would PGO help with cycle collector stuff. Or maybe we end up running CC and forgetSkippable during pgo runs.
Flags: needinfo?(bugs) → needinfo?(continuation)
Comment 5•10 years ago
|
||
Some kind of page close CC might be nice, I guess. Though I expect any PGO run that lasts for more than like 20 seconds will have CC.
Flags: needinfo?(continuation)
Comment 7•10 years ago
|
||
Did you do a full Talos run on this change? PGO is full of arbitrary heuristics, so changing the input can have unexpected results on benchmarks. In general if this is a win on some benchmarks and not a big loss on others it's fine. Last time we changed the profiling input from an empty page to the current incarnation we didn't really move the needle on our benchmarks.
Comment 8•10 years ago
|
||
Would the AWSY pageset make a useful addition? IIRC these are real world webpages modified to remove some unpredictable factors (to make them more useful for measuring memory usage). I don't know how long they take to run though (or if there are other factors that would make them unsuitable). Nicholas, what do you think?
Flags: needinfo?(n.nethercote)
Comment 9•10 years ago
|
||
AreWeSlimYet just uses Talos.
Comment 10•10 years ago
|
||
The difference between how AWSY runs the Talos pageset and how Talos runs it is basically that it just runs it very slowly so the GC and CC have time to run, which should not really affect PGO too much, except that there will be more GC and CC in the profile.
Assignee | ||
Comment 12•10 years ago
|
||
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #7) > Did you do a full Talos run on this change? PGO is full of arbitrary > heuristics, so changing the input can have unexpected results on benchmarks. Not yet because the trees were closed yesterday. Will do a Try push today.
Assignee | ||
Comment 13•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=eb7b1345012f
Comment 14•10 years ago
|
||
Comment on attachment 8519983 [details] [diff] [review] Part 2 - Add Octane Review of attachment 8519983 [details] [diff] [review]: ----------------------------------------------------------------- ::: build/pgo/index.html @@ +6,1 @@ > var list = While you are at it, can you remove all trailing spaces in this file? ::: build/pgo/js-input/octane/base.js @@ +156,5 @@ > } else { > continuation = suite.RunStep(runner); > } > } > + if (continuation && 0) {//XXX && typeof window != 'undefined' && window.setTimeout) { Can you add a comment here explaining why we don't use window.setTimeout?
Attachment #8519983 -
Flags: review?(hv1989) → review+
Updated•10 years ago
|
Attachment #8519913 -
Flags: review?(ted) → review+
Assignee | ||
Comment 15•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/dcbd5f128917 https://hg.mozilla.org/integration/mozilla-inbound/rev/a3f7920495b4 PGO Try push last week didn't show a Talos regression, but I'll keep an eye on Talos and the PGO builds on AWFY and backout if it regresses something.
https://hg.mozilla.org/mozilla-central/rev/dcbd5f128917 https://hg.mozilla.org/mozilla-central/rev/a3f7920495b4
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Assignee | ||
Comment 17•10 years ago
|
||
Backed out part 2: https://hg.mozilla.org/integration/mozilla-inbound/rev/bf36fe5a2125 It didn't help much and on AWFY it looks like some benchmark became a bit more noisy. Also, for some reason the dromaeo-object-string PGO issue was gone on a few builds, but on other runs it was about as fast as before or a bit slower even: http://arewefastyet.com/#machine=17&view=single&suite=dromaeo&subtest=dromaeo-object-string Oh well, at least we tried.
Assignee | ||
Updated•10 years ago
|
Resolution: FIXED → WONTFIX
Comment 18•9 years ago
|
||
actually winxp pgo did show a slight regression in tp5o and tp5o responsiveness, I didn't see any other regressions related to this though
Comment 19•9 years ago
|
||
I think it may be better to add more benchmarks to the PGO profile. Not only including js benchmark, but also taking the HTML5, Canvas, CSS, Dom benchmarks into account.
Comment 20•9 years ago
|
||
Actually, I found two websites which is used to collect the benchmark. One of them is the project of Mozilla on the github: https://github.com/servo/servo/wiki/Benchmarks Another onee is the IE Testdrive: http://ie.microsoft.com/testdrive/ I hope them could help you to absorb the essence of the files and add them to the PGO profile in order to make Firefox have a better performance overall.
Comment 21•9 years ago
|
||
And at this time, there is no desired expection(or it was going backawrd) after adding the Octane to profile. I suggest to analysis the reason deeply because I believe that PGO is a good way to improve the performance. Thanks for your effort.
Comment 22•9 years ago
|
||
Unfortunately real-world testing hasn't proven much value for adding more benchmarks to the PGO profile. We are certainly open to data that shows otherwise, but our experience shows that it just doesn't work the way we all think it ought to.
Comment 23•9 years ago
|
||
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #22) > Unfortunately real-world testing hasn't proven much value for adding more > benchmarks to the PGO profile. We are certainly open to data that shows > otherwise, but our experience shows that it just doesn't work the way we all > think it ought to. Sorry, I don't understand why it is unworthy adding more benchmarks to the PGO profile because I can see a better performance when I use the pcxFirefox which is a third party build Firefox. Especially, I ran some benchmarks on my laptop, such as Sunspider, Octane, Kraken, HTML5 benchmark and Speedometer. It is better than the official version in many aspects. I just know the author use BetterPGO and apply PGO to all modules. So I guess it is a good way to add them to profile. Here is the pcxFirefox & the compiler: http://sourceforge.net/projects/pcxfirefox/ https://bugzilla.mozilla.org/user_profile?user_id=402729
Flags: needinfo?(ho0thy0oy)
Comment 24•9 years ago
|
||
(In reply to ghou from comment #23) > (In reply to Ted Mielczarek [:ted.mielczarek] from comment #22) > > Unfortunately real-world testing hasn't proven much value for adding more > > benchmarks to the PGO profile. We are certainly open to data that shows > > otherwise, but our experience shows that it just doesn't work the way we all > > think it ought to. > > Sorry, I don't understand why it is unworthy adding more benchmarks to the > PGO profile because I can see a better performance when I use the pcxFirefox > which is a third party build Firefox. Especially, I ran some benchmarks on > my laptop, such as Sunspider, Octane, Kraken, HTML5 benchmark and > Speedometer. It is better than the official version in many aspects. I just > know the author use BetterPGO and apply PGO to all modules. So I guess it is > a good way to add them to profile. > Here is the pcxFirefox & the compiler: > http://sourceforge.net/projects/pcxfirefox/ > https://bugzilla.mozilla.org/user_profile?user_id=402729 This is because if we hope adding PGO tests did not cause some obvious regressions, we should not only add sunspider and octane, but also should add talos tests and so on, which will make PGO build time much longer. I think most people don't want very long build time.
Comment 25•9 years ago
|
||
(In reply to xunxun from comment #24) Firstly, thanks for answering. So I have a idea that is to do the same thing like your way when the stable version is released because of its long period of time. Perhaps, it is accepted to spend more time buliding instead of improving the performance slowly(or sometimes causing the regressions). Am I right? Secondly, I hop applying PGO to all modules, which is the way you used in the description, would be adopted in the official version. In the past, the way was limited by the memory of link, which would exceed 4G. However, as I known, there is something called unified-compilation now so that it is a nice time to use PGO overall.
Comment 26•9 years ago
|
||
I think the main reason pcxFirefox (and others) is faster is because it requires SSE2 while Firefox itself still supports non-SSE CPUs. See bug 836658 and also https://groups.google.com/d/msg/mozilla.dev.platform/GWb7C76nRqk/XIU8VmO5_tAJ
Comment 27•9 years ago
|
||
(In reply to Hugh Nougher [:Hughman] from comment #26) > I think the main reason pcxFirefox (and others) is faster is because it > requires SSE2 while Firefox itself still supports non-SSE CPUs. > > See bug 836658 and also > https://groups.google.com/d/msg/mozilla.dev.platform/GWb7C76nRqk/XIU8VmO5_tAJ Actually, there are some SSE2 codes in the part of js engine and graphics module in Firefox. But the performance couldn't be changed quite well through only building the SSE2 version. There is no doubt that PGO is a better way. I agree that Mozilla wants to support non-SSE CPUs. From another aspect, Firefox needs the lowest Windows version which is XP SP3. And if the users want to run Firefox on a normal level, they should have at least around 512MB memory, which means only if their CPUs doesn't support SSE, they may have the less memory. So defaults to SSE2 builds could be put forward nowadays. Studying from chrome. By reading the URL, I got the opinion that SSE2 isn't much better than MMX, so I think the performance of pcxFirefox mostly doesn't rely on SSE2 but is related to PGO. Let's pay our attention on whether it is useful to apply a complete PGO build in the releases version.
Comment 28•9 years ago
|
||
(In reply to Hugh Nougher [:Hughman] from comment #26) > I think the main reason pcxFirefox (and others) is faster is because it > requires SSE2 while Firefox itself still supports non-SSE CPUs. > > See bug 836658 and also > https://groups.google.com/d/msg/mozilla.dev.platform/GWb7C76nRqk/XIU8VmO5_tAJ And VC2013's Auto-Vectorizer is very conservative: http://pcxfirefox.wordpress.com/2014/10/18/vc2013-auto-vectorizer-and-auto-parallelizer-reports-on-mozilla-source-code/, so PGO is much better than SSE2, and according to MSDN, PGO also can reduce the memory use.
You need to log in
before you can comment on or make changes to this bug.
Description
•