Closed Bug 915167 Opened 7 years ago Closed 6 years ago

AWFY FxOS regressions: 6.3% Sunspider and 6.3% Kraken between Sept 6 18:55 - Sept 11 2:36

Categories

(Core :: JavaScript Engine, defect, major)

ARM
Gonk (Firefox OS)
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: nbp, Assigned: luke)

References

Details

(Keywords: perf, Whiteboard: [c=benchmark p= s=2013.10.25 u=])

Attachments

(1 file)

Note this might be caused by multiple bugs, as the Tegra only has a regression on Sunspider but not on Kraken.

The other detail is that the Unagi has only one core, and the recent change of the helperThreadCount() in Bug 912589 in which case we need to investigate if the regression is worth the responsiveness provided by the change.

http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=0452b5b504d0&tochange=5f52bbf813be
Oh, oops.  I was under the mistaken impression that somewhere else we avoided creating helper threads when GetCPUCount() == 0.  I think it makes to keep helperThreadCount() returning 0 when GetCPUCount() == 1.
Attachment #803048 - Flags: review?(bhackett1024)
Attachment #803048 - Flags: review?(bhackett1024) → review+
(In reply to Luke Wagner [:luke] from comment #1)
> Created attachment 803048 [details] [diff] [review]
> fix helperThreadCount when num cpus is 1
> 
> Oh, oops.  I was under the mistaken impression that somewhere else we
> avoided creating helper threads when GetCPUCount() == 0.  I think it makes
> to keep helperThreadCount() returning 0 when GetCPUCount() == 1.

Thanks luke for fixing this issue and adding the [leave open], as it seems that the problem persist on Sunspider.  I will try to bisect before Monday.
I tried to bisect but the harness was busted because of Bug 914671.  Currently I am doing a bisect to find what caused Bug 914671, and hopefully I should be able to bisect while reverting these changes.
Blocks: 922295
I tried to find a shorter range for the regression on B2G, but due to the harness issue, the automation is unable to get any useful benchmark results, even if I make the automation patch the source with the fix provided in Bug 913424 before testing Sunspider.

http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=0452b5b504d0&tochange=5f52bbf813be

I will try to bisect manually.
Ok, after finding the issue with the automated regression finder … I came up with the following list of changesets from the previous regression range. These run were made on a GGC build of B2G (which is showing similar results), except that octane does not run successfully.

5b8391be568d - 3458, 40278
91ba20da3b79 - 3403, 39935
1272d731770c - 3387, 40278
dc86c80d01c0 - 3435, 40082
3ade0212edd0 - 3402, 39922 <--- Sunspider & Kraken !
074ec56640f6 - 3230, 38180
9aa9c1113d5e - 3194, 38412
e112a8245e29 - 3195, 38160
8c452ca6d416 - 3262, 38298
61824642543a - 3177, 38482 
5fd5f21d4663 - 3409, 38252
ab0a4b4b9b1f - 3391, 38250 <--- Sunspider ???
19918a47a06f - 3205, 38189
255093e2f430 - 3216, 38673
4ab57d0318ff - 3182, 38330
ac51f4fe9299 - 3232, 38163

My conclusion is that this precise regression has been fixed, as the only changeset blamed for sunspider & kraken regression is the one fixed by Luke.

Also, I cannot explain the measurements obtained on ab0a4b4b9b1f.
Assignee: general → luke
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [leave open]
Keywords: perf
Whiteboard: [c=benchmark p= s=2013.10.25 u=]
Target Milestone: --- → mozilla26
(In reply to Luke Wagner [:luke] from comment #1)
> Created attachment 803048 [details] [diff] [review]
> fix helperThreadCount when num cpus is 1
> 
> Oh, oops.  I was under the mistaken impression that somewhere else we
> avoided creating helper threads when GetCPUCount() == 0.  I think it makes
> to keep helperThreadCount() returning 0 when GetCPUCount() == 1.

A good number of the jit-tests seem to assume helper threads are available, and testing on ARMv6 single core devices is frustrated by this change.  Might it be appropriate to try and address this, or would it be best to just keep on applying a local patch for testing on single core devices?
Flags: needinfo?(luke)
Bug 941827 will make --thread-count=N effectively replace GetCPUCount() with N, so --thread-count=2 should give you helper threads.
Flags: needinfo?(luke)
You need to log in before you can comment on or make changes to this bug.