If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

jsapi-tests.exe and gdb-tests.exe don't need to be PGO'd

RESOLVED FIXED in Firefox 57

Status

()

Core
Build Config
RESOLVED FIXED
a month ago
28 days ago

People

(Reporter: dmajor, Assigned: dmajor)

Tracking

unspecified
mozilla57
Points:
---

Firefox Tracking Flags

(firefox57 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

a month ago
I was running a PGO build locally and saw some "link.exe ... -LTCG:PGUPDATE" processes running for jsapi-tests.exe and gdb-tests.exe. 

This is probably unnecessary work.
(Assignee)

Comment 1

a month ago
Oh, according to my console, webrtc-gtest.exe also.

ted says this can be fixed by adding `NO_PGO = True` to each moz.build.
(Assignee)

Comment 2

a month ago
Anything in here that we don't ship is a candidate to un-PGO.

\accessible\interfaces\ia2\IA2Marshal.pgd
\accessible\interfaces\msaa\AccessibleMarshal.pgd
\accessible\ipc\win\handler\AccessibleHandler.pgd
\browser\app\firefox.pgd
\build\win32\crashinjectdll\crashinjectdll.pgd
\config\external\lgpllibs\lgpllibs.pgd
\dom\media\gmp-plugin\fake.pgd
\dom\media\gmp-plugin-openh264\fakeopenh264.pgd
\dom\plugins\ipc\hangui\plugin-hang-ui.pgd
\dom\plugins\test\testplugin\nptest.pgd
\dom\plugins\test\testplugin\flashplugin\npswftest.pgd
\dom\plugins\test\testplugin\secondplugin\npsecondtest.pgd
\dom\plugins\test\testplugin\thirdplugin\npthirdtest.pgd
\gfx\angle\src\libEGL\libEGL.pgd
\gfx\angle\src\libGLESv2\libGLESv2.pgd
\ipc\app\plugin-container.pgd
\js\src\gdb\gdb-tests.pgd
\js\src\jsapi-tests\jsapi-tests.pgd
\js\xpconnect\shell\xpcshell.pgd
\media\ffvpx\libavcodec\mozavcodec.pgd
\media\ffvpx\libavutil\mozavutil.pgd
\media\gmp-clearkey\0.1\clearkey.pgd
\media\webrtc\trunk\gtest\webrtc-gtest.pgd
\memory\replace\logalloc\logalloc.pgd
\memory\replace\logalloc\minimal\logalloc_minimal.pgd
\memory\replace\logalloc\replay\logalloc-replay.pgd
\memory\replace\replace\replace_malloc.pgd
\modules\libmar\tool\signmar.pgd
\mozglue\build\mozglue.pgd
\security\nss3.pgd
\security\manager\ssl\tests\unit\pkcs11testmodule\pkcs11testmodule.pgd
\testing\mochitest\ssltunnel\ssltunnel.pgd
\testing\tools\screenshot\screenshot.pgd
\toolkit\components\ctypes\tests\jsctypes-test.pgd
\toolkit\components\maintenanceservice\maintenanceservice.pgd
\toolkit\components\telemetry\pingsender\pingsender.pgd
\toolkit\components\telemetry\tests\modules-test.pgd
\toolkit\crashreporter\client\crashreporter.pgd
\toolkit\crashreporter\minidump-analyzer\minidump-analyzer.pgd
\toolkit\library\xul.pgd
\toolkit\library\dummydll\qipcap64.pgd
\toolkit\mozapps\update\updater\updater.pgd
\toolkit\mozapps\update\updater\updater-xpcshell\updater-xpcshell.pgd
(Assignee)

Comment 3

a month ago
Created attachment 8898923 [details] [diff] [review]
nopgo
Assignee: nobody → dmajor
Attachment #8898923 - Flags: review?(ted)
Comment on attachment 8898923 [details] [diff] [review]
nopgo

Review of attachment 8898923 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsapi-tests/moz.build
@@ +157,5 @@
>  
>  DEFINES['topsrcdir'] = '%s/js/src' % TOPSRCDIR
>  OBJDIR_PP_FILES.js.src['jsapi-tests'] += ['jsapi-tests-gdb.py.in']
> +
> +NO_PGO = True

jsapi-tests does link the JS library as a static library, but it's probably not going to be testing anything useful since it won't be rebuilt with optimizations since we don't run its binary during the profiling phase.

::: toolkit/crashreporter/minidump-analyzer/moz.build
@@ +30,5 @@
>  # Don't use the STL wrappers in the crashreporter clients; they don't
>  # link with -lmozalloc, and it really doesn't matter here anyway.
>  DISABLE_STL_WRAPPING = True
>  
> +NO_PGO = True

We do ship this binary, but it doesn't get invoked during profiling, so this probably doesn't make a difference.
Attachment #8898923 - Flags: review?(ted) → review+

Comment 5

29 days ago
Pushed by dmajor@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/109e89a7d561
Set NO_PGO on a bunch of binaries that we don't ship. r=ted
Backed out for busting Linux pgo builds:

https://hg.mozilla.org/integration/mozilla-inbound/rev/2748e299a9713836b8824b4201c8865b15a6dc81

Push with bustage: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=109e89a7d561d58e4b6ee6180391058e4f2447d1&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Build log: https://treeherder.mozilla.org/logviewer.html#?job_id=125205412&repo=mozilla-inbound

[task 2017-08-23T15:21:15.205298Z] 15:21:15     INFO -  collect2: error: ld returned 1 exit status
[task 2017-08-23T15:21:15.205669Z] 15:21:15     INFO -  /home/worker/workspace/build/src/config/rules.mk:578: recipe for target 'ssltunnel' failed
[task 2017-08-23T15:21:15.206037Z] 15:21:15     INFO -  gmake[6]: *** [ssltunnel] Error 1
[task 2017-08-23T15:21:15.206414Z] 15:21:15     INFO -  gmake[6]: Leaving directory '/home/worker/workspace/build/src/obj-firefox/testing/mochitest/ssltunnel'
[task 2017-08-23T15:21:15.206796Z] 15:21:15     INFO -  /home/worker/workspace/build/src/config/recurse.mk:73: recipe for target 'testing/mochitest/ssltunnel/target' failed
[task 2017-08-23T15:21:15.207168Z] 15:21:15     INFO -  gmake[5]: *** [testing/mochitest/ssltunnel/target] Error 2
[task 2017-08-23T15:21:15.207527Z] 15:21:15     INFO -  gmake[5]: *** Waiting for unfinished jobs....
Flags: needinfo?(dmajor)
Huh, maybe NO_PGO doesn't actually work on GCC builds, since anything else that gets linked in that *was* compiled with PGO flags will expect to be linked against gcov?
(Assignee)

Comment 8

29 days ago
Yeah something like that.

I don't really have the expertise or desire to dig into this myself. I'm going to find the maximal set of things I can get away with NO_PGO'ing, and move on.

Comment 9

29 days ago
Pushed by dmajor@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5e813b0d6a3c
Set NO_PGO on a bunch of binaries that we don't ship. r=ted
(Assignee)

Updated

28 days ago
Flags: needinfo?(dmajor)

Comment 10

28 days ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5e813b0d6a3c
Status: NEW → RESOLVED
Last Resolved: 28 days ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.