Closed Bug 1387436 Opened 2 years ago Closed 2 years ago

93.66 - 102.68% Strings PerfStripCharsCRLF / Strings PerfStripCharsWhitespace (osx-10-10) regression on push 2cd8c7f13e6c5ace6955da85a4d95f6e65caad2e (Mon Jul 31 2017)

Categories

(Core :: Networking, defect)

55 Branch
defect
Not set

Tracking

()

RESOLVED WORKSFORME
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- fixed
firefox56 --- unaffected
firefox57 --- unaffected

People

(Reporter: igoldan, Unassigned)

References

Details

(Keywords: perf, regression)

We have detected a platform microbenchmarks regression from push:

https://hg.mozilla.org/releases/mozilla-beta/pushloghtml?changeset=2cd8c7f13e6c5ace6955da85a4d95f6e65caad2e

As author of one of the patches included in that push, we need your help to address this regression.

Regressions:

103%  Strings PerfStripCharsCRLF osx-10-10 opt      511,916.08 -> 1,037,534.67
 94%  Strings PerfStripCharsWhitespace osx-10-10 opt 436,101.58 -> 844,567.42


You can find links to graphs and comparison views for each of the above tests at: https://treeherder.mozilla.org/perf.html#/alerts?id=8552

On the page above you can see an alert for each affected platform as well as a link to a graph showing the history of scores for this test. There is also a link to a treeherder page showing the jobs in a pushlog format.

To learn more about the regressing test(s), please see: https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Automated_Performance_Testing_and_Sheriffing/Platform_Microbenchmarks
This is an uplifted patch, consisting of two separate bugs: bug 1369571 and bug 1376459, one of which is classified.
Can you conclude which one of the two is responsible for this regression, and then estimate a fix time for it?
Flags: needinfo?(schien)
Flags: needinfo?(honzab.moz)
Blocks: 1373320
No longer blocks: 1386631
Component: Untriaged → Networking
Product: Firefox → Core
(In reply to Ionuț Goldan [:igoldan], Performance Sheriffing from comment #0)
> We have detected a platform microbenchmarks regression from push:
> 
> https://hg.mozilla.org/releases/mozilla-beta/
> pushloghtml?changeset=2cd8c7f13e6c5ace6955da85a4d95f6e65caad2e
> 
> As author of one of the patches included in that push, we need your help to
> address this regression.
> 
> Regressions:
> 
> 103%  Strings PerfStripCharsCRLF osx-10-10 opt      511,916.08 ->
> 1,037,534.67
>  94%  Strings PerfStripCharsWhitespace osx-10-10 opt 436,101.58 -> 844,567.42
> 

I first need to understand *exactly* what these tests do (maybe even to see their code).  The document at the bottom says - ehm - nothing.

My patch does some string comparing but only in edge cases (an appcached page with FALLBACK namespace) but doesn't change AT ALL any low level string code.  

Note that very similar (nearly identical) patch has landed few weeks ago on m-c and there were no regression at all.  This has also landed on esr52, is there a similar regression detected or we don't have data for it?

Thanks.
Flags: needinfo?(honzab.moz) → needinfo?(ionut.goldan)
(I'll rather keep me ni?'ed)
Flags: needinfo?(honzab.moz)
:milan added those tests in bug 1358297, he could explain what those tests measure.
Flags: needinfo?(ionut.goldan) → needinfo?(milan)
I don't see how these patches would make a difference to those tests.  They are testing low level code in xpcom/string, and the patches above hit much higher than that.
Is this PGO?  Maybe that accounts for it?
Flags: needinfo?(milan)
these regressions are on osx which we don't have a pgo option for.

In this case there was a matching improvement a couple days earlier, although this doesn't seem like a landing/backout type of situation:
https://treeherder.mozilla.org/perf.html#/graphs?timerange=7776000&series=%5Bmozilla-beta,5402e9659592ee545df2037ed7e152889daa9a7e,1,6%5D&selected=%5Bmozilla-beta,5402e9659592ee545df2037ed7e152889daa9a7e,234105,304515170,6%5D
Something weird for sure, but I don't think the above patches made anything worse.
My patch doesn't touch our string implementation as well, doesn't look like it can make any difference on this performance test.
Dropping r on me, this seems not to be related to the patches but something else.
Flags: needinfo?(honzab.moz)
It looks to me like this "regression" was the tests returning to normal after an unexplained improvement a couple days earlier. I don't think this is worth tracking.

Also note that this change occurred on Firefox 55, not 56. Changing the blocking bug accordingly.
Blocks: 1346783
No longer blocks: 1373320
Status: NEW → RESOLVED
Closed: 2 years ago
Flags: needinfo?(schien)
Resolution: --- → WORKSFORME
Version: unspecified → 55 Branch
You need to log in before you can comment on or make changes to this bug.