Talos has detected a Firefox performance regression from push 06cc94fa4cdf. 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=849 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 (add --e10s to run tests in e10s mode) 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

It is impossible for bug 1245701 to change Firefox binaries. Either the test or VC2015 PGO is totally bogus.

here are the bytes we are reading now during startup (we didn't adjust any of our other fileIO): C:\slave\test\build\application\firefox\msvcp140.dll 917504 C:\slave\test\build\application\firefox\vcruntime140.dll 262144 C:\slave\test\build\application\firefox\api-ms-win-crt-convert-l1-1-0.dll 131072 C:\slave\test\build\application\firefox\api-ms-win-crt-environment-l1-1-0.dll 131072 C:\slave\test\build\application\firefox\api-ms-win-crt-filesystem-l1-1-0.dll 131072 C:\slave\test\build\application\firefox\api-ms-win-crt-heap-l1-1-0.dll 131072 C:\slave\test\build\application\firefox\api-ms-win-crt-locale-l1-1-0.dll 131072 C:\slave\test\build\application\firefox\api-ms-win-crt-math-l1-1-0.dll 131072 C:\slave\test\build\application\firefox\api-ms-win-crt-multibyte-l1-1-0.dll 131072 C:\slave\test\build\application\firefox\api-ms-win-crt-runtime-l1-1-0.dll 131072 C:\slave\test\build\application\firefox\api-ms-win-crt-stdio-l1-1-0.dll 131072 C:\slave\test\build\application\firefox\api-ms-win-crt-string-l1-1-0.dll 131072 C:\slave\test\build\application\firefox\api-ms-win-crt-time-l1-1-0.dll 131072 C:\slave\test\build\application\firefox\api-ms-win-crt-utility-l1-1-0.dll 131072 :mshal, is this intended? if so, can you briefly explain why it is happening? if it isn't intended, can you own solving this?

:emk, this data and regressions is not related to PGO, this affects opt only. pgo doesn't seem to have an issue: https://treeherder.mozilla.org/perf.html#/graphs?series=%5Bmozilla-inbound,629fcd2674dfa38ed06ae0809f904f2d36441c6a,0%5D&series=%5Bmozilla-inbound,ee81b0a5879b3f07c993f26ad9202787df2f4de4,0%5D&series=%5Bmozilla-inbound,613690be0732171b0ac3447c139101d41fa4dc85,1%5D&series=%5Bmozilla-inbound,12945b0b398d773c03a01f2a8eb7006e2be4ca82,1%5D

I'll take a look - it's not intended. The patch was just supposed to specify the DLLs to copy in moz.build instead of the Makefile, but the set of DLLs to copy should be the same in all cases. I guess it's not :)

The old build has no CRT/UCRT entries in dependentlibs.list, but the new build has some (but not all) entries.

Ahh, thanks for pointing that out emk! Here's what's going on: In opt builds, previously we created dependentlibs.list before installing these DLLs into dist/bin. Since the DLLs don't exist, they aren't added into the list. Now, the DLL installation happens much earlier in the build, so by the time we create dependentlibs.list the DLLs exist and they are included. The reason PGO builds don't exhibit this behavior is because they do both of these steps twice, so prior to my patch it looked something like this: 1) Create dependentlibs.list (doesn't include api-ms-win*.dll, MSVCP140.dll, etc) 2) Copy DLLs into dist/bin 3) Create dependentlibs.list (which now includes api-ms-win*.dll, etc, since they are in dist/bin) 4) Copy DLLs into dist/bin Changing it so it does the copy before creating dependentlibs.list doesn't affect the final result in PGO, since the dependentlibs.list that we end up with is the one created after the copy. So I think it's likely that we'll want the DLLs in dependentlibs.list regardless of whether we're doing opt or PGO builds, meaning we'll have to eat the regression. In other words, I think it was a bug that we were creating the list without these files. Alternatively, if we really *don't* want these DLLs in dependentlibs.list, then we should remove them from both opt and PGO builds by fixing up dependentlibs.py to ignore them when creating the list. :emk, do you have a suggestion here? Or can you direct me to someone who would be knowledgeable about what should be in dependentlibs.list?

We discussed this in the build meeting. Since this is an opt-only regression, and it just makes the opt behavior match the existing PGO behavior, I think we shouldn't spend time on this. We're only shipping PGO builds to users, so that's what matters.

dependentlibs.list was introduced to help some embedding scenarios (bug 298044). In this particular case, I'm not sure we need to preload api-ms-win*.dll explicitly. SetDllDirectory() and LOAD_WITH_ALTERED_SEARCH_PATH should do the job. Benjamin might have more insights.

SetDllDirectory/ALTERED_SEARCH_PATH don't work on all our supported platforms, as I understand it. Although we no longer ship anything that uses embedding so there's no automated testing at all.

(In reply to Benjamin Smedberg [:bsmedberg] from comment #11) > SetDllDirectory/ALTERED_SEARCH_PATH don't work on all our supported > platforms, as I understand it. Although we no longer ship anything that uses > embedding so there's no automated testing at all. They *did* not work on all our supported platforms when bug 298044 was filed, no? SetDllDirectory is available XP SP1+ and LOAD_WITH_ALTERED_SEARCH_PATH is available even with Win95/NT.

WONTFIXing this per #c9. Feel free to open a new bug for the SetDllDirectory stuff if it's worthwhile, but that is outside the scope of this bug.

thanks for resolving this!