Closed Bug 1412932 Opened 2 years ago Closed 2 years ago

Move PGO logic out of client.mk

Categories

(Firefox Build System :: General, enhancement)

enhancement
Not set

Tracking

(firefox58 fixed)

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: gps, Assigned: gps)

References

(Blocks 2 open bugs)

Details

Attachments

(6 files)

Currently, the "perform a PGO build" logic is implemented in client.mk. With client.mk going away, we need a new home for this logic.

I imagine I'll just extract things to another make file or shell script as the quickest possible solution.
About --enable-pgo: I'd leave it as is, that is: leave it as MOZ_PGO. Then you can just switch mk_add_options to ac_add_options in the mozconfigs and be done with it. (yes, ac_add_options MOZ_PGO=1 works)
Comment on attachment 8923597 [details]
Bug 1412932 - Don't go through client.mk for maybe_clobber_profiledbuild;

https://reviewboard.mozilla.org/r/194728/#review202502
Attachment #8923597 - Flags: review+
Comment on attachment 8923598 [details]
Bug 1412932 - Move PGO logic from client.mk into root Makefile.in;

https://reviewboard.mozilla.org/r/194730/#review202510
Attachment #8923598 - Flags: review+
Attachment #8923598 - Flags: review?(core-build-config-reviews)
Attachment #8923597 - Flags: review?(core-build-config-reviews)
Comment on attachment 8923599 [details]
Bug 1412932 - Remove realbuild target from client.mk;

https://reviewboard.mozilla.org/r/194732/#review202512
Attachment #8923599 - Flags: review+
Attachment #8923599 - Flags: review?(core-build-config-reviews)
Comment on attachment 8923600 [details]
Bug 1412932 - Switch to PGO build in Makefile.in;

https://reviewboard.mozilla.org/r/194734/#review202520

::: Makefile.in:229
(Diff revision 1)
>  
>  # PGO build target.
>  profiledbuild::
>  	$(call BUILDSTATUS,TIERS pgo_profile_generate pgo_package pgo_profile pgo_clobber pgo_profile_use)
>  	$(call BUILDSTATUS,TIER_START pgo_profile_generate)
> -	$(MAKE) MOZ_PROFILE_GENERATE=1 MOZ_PGO_INSTRUMENTED=1
> +	$(MAKE) MOZ_PROFILE_GENERATE=1 MOZ_PGO_INSTRUMENTED=1 default

Nitpicky, but the other make invocations seem to have the target first and the variables last.

::: Makefile.in:236
(Diff revision 1)
>  	$(call BUILDSTATUS,TIER_START pgo_package)
>  	$(MAKE) package MOZ_PGO_INSTRUMENTED=1 MOZ_INTERNAL_SIGNING_FORMAT= MOZ_EXTERNAL_SIGNING_FORMAT=
>  	rm -f jarlog/en-US.log
>  	$(call BUILDSTATUS,TIER_FINISH pgo_package)
>  	$(call BUILDSTATUS,TIER_START pgo_profile)
>  	MOZ_PGO_INSTRUMENTED=1 JARLOG_FILE=jarlog/en-US.log EXTRA_TEST_ARGS=10 $(MAKE) pgo-profile-run

I just realized that the `pgo-profile-run` implementation could be inlined here at this point.
Attachment #8923600 - Flags: review+
Comment on attachment 8923601 [details]
Bug 1412932 - Set MOZ_PGO for configure;

https://reviewboard.mozilla.org/r/194736/#review202522

I don't have strong feelings about --enable-pgo vs. MOZ_PGO, honestly. If you keep the `env='MOZ_PGO'` bit then either way should work, so it's not terribly concerning.
Attachment #8923601 - Flags: review+
Attachment #8923600 - Flags: review?(core-build-config-reviews)
Attachment #8923601 - Flags: review?(core-build-config-reviews)
Comment on attachment 8926139 [details]
Bug 1412932 - Inline pgo-profile-run target;

https://reviewboard.mozilla.org/r/197376/#review202556
Attachment #8926139 - Flags: review?(ted) → review+
Attachment #8923599 - Flags: review?(core-build-config-reviews)
Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/076a93ecab10
Don't go through client.mk for maybe_clobber_profiledbuild; r=ted
https://hg.mozilla.org/integration/autoland/rev/818df7ceccbf
Move PGO logic from client.mk into root Makefile.in; r=ted
https://hg.mozilla.org/integration/autoland/rev/8c8777aae67f
Remove realbuild target from client.mk; r=ted
https://hg.mozilla.org/integration/autoland/rev/b5f79b0f2a35
Switch to PGO build in Makefile.in; r=ted
https://hg.mozilla.org/integration/autoland/rev/5ce23bfbb798
Inline pgo-profile-run target; r=ted
https://hg.mozilla.org/integration/autoland/rev/2013c8dd1824
Set MOZ_PGO for configure; r=ted
Blocks: 1416010
Blocks: 1320738
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.