1262492 - 2.6% tp5n main_startup_fileio (windows7-32) regression on push b65d504944ef (Tue Apr 5 2016)

Status: RESOLVED WONTFIX

(Firefox Build System :: General, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

Talos has detected a Firefox performance regression from push b65d504944ef. As author of one of the patches included in that push, we need your help to address this regression.

This is a list of all known regressions and improvements related to the push:
https://treeherder.mozilla.org/perf.html#/alerts?id=751

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 Talos jobs in a pushlog format.

To learn more about the regressing test(s), please see:
https://wiki.mozilla.org/Buildbot/Talos/Tests#xperf

Reproducing and debugging the regression:

If you would like to re-run this Talos test on a potential fix, use try with the following syntax:
try: -b o -p win32 -u none -t xperf[Windows 7] --rebuild 5  # add "mozharness: --spsProfile" to generate profile data

(we suggest --rebuild 5 to be more confident in the results)

To run the test locally and do a more in-depth investigation, first set up a local Talos environment:
https://wiki.mozilla.lorg/Buildbot/Talos/Running#Running_locally_-_Source_Code

Then run the following command from the directory where you set up Talos:
talos --develop -e [path]/firefox -a tp5n

Making a decision:
As the patch author we need your feedback to help us handle this regression.
*** Please let us know your plans by Monday, or the offending patch(es) will be backed out! ***

Our wiki page outlines the common responses and expectations:
https://wiki.mozilla.org/Buildbot/Talos/RegressionBugsHandling

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

:ted, can you look into this and confirm that your changes should have incrased the startup fileio measured?

Comment 2

[(not currently active) Ted Mielczarek]

Hi! I'm at TRIBE today and tomorrow so I won't have time to look into this just yet. This doesn't entirely surprise me, but I'd like to make sure that the measurement is actually measuring something sensible. With those patches, we changed from building the ~10MB of ICU data into a standalone shared library that libxul would link against to shipping that data in a separate file (icudt56l.dat), which ICU presumably opens on startup (as part of JS_Init).

On non-Windows, we were linking that data directly into libxul, so we were certainly loading the data on startup since we preload libxul during startup. On Windows the first of those two patches also changed things so that instead of building ICU as a set of shared libraries we build it as static libraries that get linked into libxul. With the combination of those two patches I could believe that we wind up loading more data during startup, depending on ICU's data-loading behavior.

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

thanks for the info ted, given the fact that we increased the startup fileio by slightly >10MB, that aligns well with your analysis here.  I suspect we didn't load all of libxul before, or it was preloaded by the OS as you pointed out.  Now we have a new file which isn't preloaded and we have to load it all.

xperf should be measuring all preloaded (cached) as well as new (disk) access in the startup.  After TRIBE maybe we can dig into this slightly.

This doesn't seem to affect startup of the browser, and I see this fileio increase on pgo:
https://treeherder.mozilla.org/perf.html#/graphs?series=%5Bmozilla-inbound,629fcd2674dfa38ed06ae0809f904f2d36441c6a,1%5D&series=%5Bmozilla-inbound,613690be0732171b0ac3447c139101d41fa4dc85,1%5D&selected=%5Bmozilla-inbound,629fcd2674dfa38ed06ae0809f904f2d36441c6a,NaN,null%5D

the good news is that for firefox 48 the net increase is a win since we had a larger win in late march (I believe vs2015)

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

this also seems to have caused a slight regression in num_constructors:
https://treeherder.mozilla.org/perf.html#/alerts?id=755

and the corresponding graph:
https://treeherder.mozilla.org/perf.html#/graphs?series=%5Bfx-team,429db73edbc2360aa43f534977984a2e3e64bda6,1%5D&series=%5Bmozilla-inbound,429db73edbc2360aa43f534977984a2e3e64bda6,1%5D&series=%5Bmozilla-central,429db73edbc2360aa43f534977984a2e3e64bda6,1%5D&zoom=1459833534459.144,1459979040980.5447,80,92.0817843866171&selected=%5Bmozilla-inbound,429db73edbc2360aa43f534977984a2e3e64bda6,29582,25197437%5D

Comment 5

[(not currently active) Ted Mielczarek]

I think these are both fallout from bug 1247396, which just happened to land together with the other patch. Before that patch we built ICU as standalone shared libraries on Windows, now we're linking that code into libxul.

Comment 6

[Joel Maher ( :jmaher ) (UTC -8)]

pgo results are in, for the most part identical.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

This sounds like a WONTFIX?