GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — gh-128515: Add BOLT build to CI by zanieb · Pull Request #128845 · python/cpython Conversation
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Interesting, edit: This occurs because |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
I encountered a couple blockers for aarch64, a failed assertion in the instrumented binary and (after hacking past that) a segfault in BOLT I dropped aarch64 in 684ece4 — we can add it later. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
A few tests are failing after BOLT optimization. I'd appreciate some guidance on that. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
The timing on this actually seems pretty reasonable at 13 minutes. We could expand this to perform other build optimizations, e.g., PGO, to verify they're working as intended? Right now it's just BOLT though. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Two things
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Can you expand on this comment?
Sounds good to me — should I open an issue to investigate why they fail too? Like is the profiler actually broken? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Because we skip several tests with BOLTed binary, PGO + LTO can not check the regression issue where tests are skipped. Currently, PGO + LTO is the standard optimization policy of the CPython project.
Yeah, we should; maybe @pablogsal is interested in this issue. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| # Do not test BOLT with free-threading, to conserve resources | ||
| - bolt: true | ||
| free-threading: true | ||
| # BOLT currently crashes during instrumentation on aarch64 | ||
| - os: ubuntu-24.04-aarch64 | ||
| bolt: true |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I don't have strong feelings about this pattern (using exclude instead of include), but liked that I could document why we're not running the additional cases.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| @@ -246,10 +250,17 @@ jobs: | |||
| exclude: | |||
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Not a strong opinion, but I would prefer to have just 1, 2, or 3 jobs with bolt. unless it is absolutely needed.
We can move some very specific builds to buildbots, while maintatining the bare minimum in CI.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
This is just one job with BOLT — I think in the future we'd want a second job for aarch64 once that's unblocked. Are you suggesting I should frame this as an include instead? ref #128845 (comment)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Yes! Sorry for not being clear :)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I'll wait to change it until others have a chance to weigh in, but I'm not opposed.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I started to do this but found it awkward since I know we want aarch64 testing here eventually. Since @hugovk 👍 my comment at #128845 (comment) I think I'll leave it for now.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| # `test_unpickle_module_race` writes to the source directory, which is | ||
| # read-only during builds — so we exclude it from profiling with BOLT. | ||
| run: >- | ||
| PROFILE_TASK='-m test --pgo --ignore test_unpickle_module_race' |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Interesting, why doesn't it raise any issues while we build PGO build?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I don't think there is a PGO build in CI
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
(The read-only build file system looks specific to this CI setup)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I don't think there is a PGO build in CI
FYI, At Github Action there is no CI for PGO and LTO.
But at build bot we run CI for the PGO and LTO.
https://buildbot.python.org/#/builders/378/builds/1554
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
(The read-only build file system looks specific to this CI setup)
Let me take a look more detail. :)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/python/cpython/actions/runs/12776586124/job/35615597427
ERROR: test_unpickle_module_race (test.test_pickle.PyUnpicklerTests.test_unpickle_module_race)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/runner/work/cpython/cpython-ro-srcdir/Lib/test/support/import_helper.py", line 48, in forget
unlink(source + 'c')
~~~~~~^^^^^^^^^^^^^^
File "/home/runner/work/cpython/cpython-ro-srcdir/Lib/test/support/os_helper.py", line 345, in unlink
_unlink(filename)
~~~~~~~^^^^^^^^^^
OSError: [Errno 30] Read-only file system: '/home/runner/work/cpython/cpython-ro-srcdir/Lib/locker.pyc'
I think it's fine to remove the test from the optimization suite. It seems likely for there to be some problems here due to the read-only setup. It's known that the tests require a writeable source directory
cpython/.github/workflows/reusable-ubuntu.yml
Lines 105 to 110 in ae31df3
| - name: Remount sources writable for tests | |
| # some tests write to srcdir, lack of pyc files slows down testing | |
| run: sudo mount "$CPYTHON_RO_SRCDIR" -oremount,rw | |
| - name: Tests | |
| working-directory: ${{ env.CPYTHON_BUILDDIR }} | |
| run: xvfb-run make ci |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
#29904 added the read-only out of tree builds.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
(We could disable the read-only builds for the BOLT job but it seems more painful than it's worth)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Ah okay let’s exclude the test with current way and let’s pile the issue about the test suite problem. Thank you for the investigation:)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
@hugovk Do we have any reason to mount file system as the read-only?
Zanie found it was added in #29904, which gives this reason:
The Ubuntu test runner now configures and compiles CPython out of tree.
The source directory is a read-only bind mount to ensure that the build
cannot create or modify any files in the source tree.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
| # `test_unpickle_module_race` writes to the source directory, which is | ||
| # read-only during builds — so we exclude it from profiling with BOLT. | ||
| run: >- | ||
| PROFILE_TASK='-m test --pgo --ignore test_unpickle_module_race' |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
@hugovk Do we have any reason to mount file system as the read-only?
Zanie found it was added in #29904, which gives this reason:
The Ubuntu test runner now configures and compiles CPython out of tree.
The source directory is a read-only bind mount to ensure that the build
cannot create or modify any files in the source tree.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
lgtm! Thanks you for the work!
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.