Closed
Bug 1056529
Opened 10 years ago
Closed 10 years ago
[BaselineCompiler] Sunspider-math-cordic takes 53% more time on windows
Categories
(Core :: JavaScript Engine: JIT, defect)
Tracking
()
RESOLVED
FIXED
mozilla34
People
(Reporter: h4writer, Assigned: jandem)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
8.24 KB,
patch
|
h4writer
:
review+
|
Details | Diff | Splinter Review |
When running ss-math-cordic with --no-ion I get the following difference on Windows/Linux:
Windows: 47.1ms
Linux: 30.6ms
I found already what the issue can be:
http://dxr.mozilla.org/mozilla-central/source/js/src/jit/BaselineIC.cpp#2506
http://dxr.mozilla.org/mozilla-central/source/js/src/jit/BaselineIC.cpp#3072
We disabled some optimizations for windows because of PGO.
Reporter | ||
Comment 1•10 years ago
|
||
PGO is currently disabled on the JS directory. Shouldn't we have removed all those flags at the same time. Since they disable all optimizations on those functions and it hits us.
Just grepping for _MSC_VER and PGO shows a lot of places we can remove this.
Blocks: 1028242
Flags: needinfo?(jdemooij)
Reporter | ||
Comment 2•10 years ago
|
||
E.g. on Raytrace we spend 6% in epowi on windows, while we only spend 3% in linux
http://dxr.mozilla.org/mozilla-central/source/js/src/jsmath.cpp#636
Removing this, gives us the same amount of time spend in epowi on windows.
Note: this only improve our score marginally on raytrace. But it could help other benchmarks much more.
Assignee | ||
Comment 3•10 years ago
|
||
(In reply to Hannes Verschore [:h4writer] from comment #1)
> PGO is currently disabled on the JS directory. Shouldn't we have removed all
> those flags at the same time. Since they disable all optimizations on those
> functions and it hits us.
PGO is only disabled for MSVC 2010. Although I doubt it, it's possible newer versions still have some of these bugs. We could disable the workarounds for MSVC 2010 though, and file a followup bug to recheck once we update the compiler...
How much does math-cordic improve if you remove this?
Flags: needinfo?(jdemooij)
Reporter | ||
Comment 4•10 years ago
|
||
(In reply to Jan de Mooij [:jandem] from comment #3)
> How much does math-cordic improve if you remove this?
We get the same speed as linux in that case. So windows improves from 47.1ms to +-30.6ms.
Assignee | ||
Comment 5•10 years ago
|
||
(In reply to Hannes Verschore [:h4writer] from comment #4)
> We get the same speed as linux in that case. So windows improves from 47.1ms
> to +-30.6ms.
Wow, good find!
Btw, if fallback stub performance matters so much on this test, is it possible we're not attaching a stub for some operation?
Reporter | ||
Comment 6•10 years ago
|
||
(In reply to Hannes Verschore [:h4writer] from comment #2)
> E.g. on Raytrace we spend 6% in epowi on windows, while we only spend 3% in
> linux
> http://dxr.mozilla.org/mozilla-central/source/js/src/jsmath.cpp#636
>
> Removing this, gives us the same amount of time spend in epowi on windows.
> Note: this only improve our score marginally on raytrace. But it could help
> other benchmarks much more.
I measured this again. And I saw an improvement of 5% - 7% when removing that.
Assignee | ||
Comment 7•10 years ago
|
||
This patch removes all pragmas we added to work around PGO bugs. If we run into PGO bugs with MSVC 2013 we can just reverse this patch, but from what I've seen MSVC 2013 PGO is a lot more mature.
Windows Try push: https://tbpl.mozilla.org/?tree=Try&rev=e714d021ce82
Windows PGO Try push: https://tbpl.mozilla.org/?tree=Try&rev=6ac9d6bbadad
Reporter | ||
Updated•10 years ago
|
Attachment #8479292 -
Flags: review?(hv1989) → review+
Reporter | ||
Comment 8•10 years ago
|
||
(In reply to Jan de Mooij [:jandem] from comment #5)
> (In reply to Hannes Verschore [:h4writer] from comment #4)
> > We get the same speed as linux in that case. So windows improves from 47.1ms
> > to +-30.6ms.
>
> Btw, if fallback stub performance matters so much on this test, is it
> possible we're not attaching a stub for some operation?
Will you open a follow-up bug for this?
Assignee | ||
Comment 9•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/a2a10e38f42d
(In reply to Hannes Verschore [:h4writer] from comment #8)
> Will you open a follow-up bug for this?
Yes, will file in a bit.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in
before you can comment on or make changes to this bug.
Description
•