Repository navigation
[HOLD on report-only PGO plan] [No QA] Reference native PGO CI preview - #102434
Draft
chrispader wants to merge 66 commits into
Draft
chrispader wants to merge 66 commits into
chrispader wants to merge 66 commits into
Conversation
Contributor
|
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Privileged PGO preview jobs need trusted physical runners and repository build secrets. This draft retains the earlier preview prototype and inherits the iOS instrumentation and proof fixes. It also opts deployments into optimized release builds, so it is outside the report-only merge plan at margelo#32.
Release PGO training needs signed-in physical devices and build secrets, which a PR from the Margelo fork cannot access in Expensify/App. This draft brings the native PGO work to Expensify main for review and adds a workflow for a trusted branch inside Expensify/App. Once a maintainer mirrors this branch there and opens a same-repository PR, each push will train a profile, build Release artifacts, and run paired benchmarks without submitting an app to a store.
Explanation of Change
The inherited iOS corrections cover compatible IR instrumentation, explicit Swift frontend flags, Rock argument quoting and sealed IPA proof verification before upload. Local iOS verification was a build/profile/startup smoke on a pinned precursor snapshot; it was not a paired performance benchmark or a GitHub Actions/store-export run.
The preview calls the Android and iOS Release build jobs with PGO and benchmarking enabled. It uploads the build proof and benchmark CSV files with a reduced manifest that excludes the account email and device ID. Ordinary PR builds keep their existing runners; the deploy workflow explicitly opts into PGO so its Release behavior remains the same.
The preview runs only when the PR head is
@chrispader/perf/pgo-ci-previewin Expensify/App. This fork PR cannot run the privileged preview. The physical-device runners and approved account fixture still need to be provisioned, and production promotion still needs a proof gate before this work is ready to merge. The existing Margelo CI draft is margelo#31.Fixed Issues
$ #98008
PROPOSAL: #98008
The tracking issue contains the proposal. Formal proposal approval remains pending.
MOBILE-EXPENSIFY: https://github.com/Expensify/Mobile-Expensify/pull/14046
Tests
Offline tests
QA Steps
This PR changes native build infrastructure only. QA cannot verify that the profile was applied to a native binary through the staging app UI, so the title carries the No QA prefix.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
No app UI changes are included. Screenshot preference is pending.