Skip to content

Always build PGO using RUSTFLAGS #739

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 4, 2023
Merged

Always build PGO using RUSTFLAGS #739

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dadbc62
Always build PGO using `RUSTFLAGS`
messense Jul 4, 2023
aac3354
revert llvm-profdata change
messense Jul 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,9 +454,10 @@ jobs:
--release
--out pgo-wheel
--interpreter ${{ matrix.maturin-interpreter || matrix.interpreter }}
-- -Cprofile-generate=${{ github.workspace }}/profdata
rust-toolchain: stable
docker-options: -e CI
env:
RUSTFLAGS: "-Cprofile-generate=${{ github.workspace }}/profdata"

- name: detect rust host
run: echo RUST_HOST=$(rustc -Vv | grep host | cut -d ' ' -f 2) >> "$GITHUB_ENV"
Expand All @@ -468,10 +469,9 @@ jobs:
pip install -r tests/requirements.txt
pip install pydantic-core --no-index --no-deps --find-links pgo-wheel --force-reinstall
pytest tests/benchmarks
rustup run stable bash -c 'echo LLVM_PROFDATA=$RUSTUP_HOME/toolchains/$RUSTUP_TOOLCHAIN/lib/rustlib/${{ env.RUST_HOST }}/bin/llvm-profdata >> "$GITHUB_ENV"'

- name: merge pgo data
run: ${{ env.LLVM_PROFDATA }} merge -o ${{ github.workspace }}/merged.profdata ${{ github.workspace }}/profdata
run: rustup run stable llvm-profdata merge -o ${{ github.workspace }}/merged.profdata ${{ github.workspace }}/profdata
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Odd, worked for me locally on a Linux server.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah llvm-profdata is in an unusual bin directory (see the way I figure it out just above). The rustup team might have knowledge of why that worked (and the best practices to add it to path).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rust-lang/rustup#3397


- name: build pgo-optimized wheel
uses: PyO3/maturin-action@v1
Expand All @@ -482,9 +482,10 @@ jobs:
--release
--out dist
--interpreter ${{ matrix.maturin-interpreter || matrix.interpreter }}
-- -Cprofile-use=${{ github.workspace }}/merged.profdata
rust-toolchain: stable
docker-options: -e CI
env:
RUSTFLAGS: "-Cprofile-use=${{ github.workspace }}/merged.profdata"

- run: ${{ matrix.ls || 'ls -lh' }} dist/

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/codspeed.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ jobs:
run: pytest tests/benchmarks

- name: Prepare merged pgo data
run: rustup run stable bash -c '$RUSTUP_HOME/toolchains/$RUSTUP_TOOLCHAIN/lib/rustlib/x86_64-unknown-linux-gnu/bin/llvm-profdata merge -o ${{ github.workspace }}/merged.profdata ${{ github.workspace }}/profdata'
run: rustup run stable llvm-profdata merge -o ${{ github.workspace }}/merged.profdata ${{ github.workspace }}/profdata

- name: Compile pydantic-core for benchmarking
# --no-default-features to avoid using mimalloc
Expand Down