Skip to content

Add PGO builds on Windows to buildbots #560

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.

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

Closed
mdboom opened this issue Nov 22, 2024 · 9 comments
Closed

Add PGO builds on Windows to buildbots #560

mdboom opened this issue Nov 22, 2024 · 9 comments
Assignees

Comments

@mdboom
Copy link

mdboom commented Nov 22, 2024

Feature or enhancement

Proposal:

We currently don't have any build bot coverage that tests PGO on Windows. This means we land code that breaks the Windows release build not all that infrequently (examples over the last 12 months: python/cpython#113655, python/cpython#125217, python/cpython#123153, python/cpython#122921).

It would be nice to add this to existing buildbots, but I know our Windows buildbots are few, and PGO builds are time consuming. Cc'ing the Windows buildbot owners for discussion: @ambv, @itamaro, @db3l.

As an aside, there is plenty of PGO coverage on the Linux buildbots, but that's largely irrelevant to the problem as it's an entirely different compiler toolchain. There is no PGO coverage on macOS either, but I have never seen that break -- if we want that for completeness, I suggest we file another issue to track that.

Has this already been discussed elsewhere?

No response given

Links to previous discussion of this feature:

No response

@itamaro
Copy link
Contributor

itamaro commented Nov 22, 2024

I can try adding PGO windows free threaded builder, initially "unstable".
Is there an example builder configuration that I can copy from? Is it just adding a --pgo flag?

@mdboom
Copy link
Author

mdboom commented Nov 22, 2024

Is it just adding a --pgo flag?

Yes. Probably should add -c Release so it's the same as python.org builds.

@db3l
Copy link

db3l commented Nov 22, 2024

Glancing at the PCbuild\build.bat file "--pgo" appears to override -c (and -d) as it builds with two fixed configurations (PGInstrument then PGUpdate) rather than Release or Debug. Presumably the PG* configs are based on Release, though I haven't actually looked, but I think just adding "--pgo" should be sufficient

In terms of overhead, perhaps a single PGO builder just for main/3.x to start with would help. It looks like all of the referenced issues showed up on main, so that might provide the most initial bang for the buck.

I do want to run a quick test build (once there's a gap in the active queue) to see what the actual build-time overhead is on my worker. For my worker at least, it's dedicated, so the only question of overhead is still being able to keep up with the normal build queue.

@mdboom
Copy link
Author

mdboom commented Nov 22, 2024

In terms of overhead, perhaps a single PGO builder just for main/3.x to start with would help. It looks like all of the referenced issues showed up on main, so that might provide the most initial bang for the buck.

Yes, main seems to matter the most by a long shot.

@db3l
Copy link

db3l commented Nov 22, 2024

Ok, so at least on my worker, it appears a PGO build shouldn't be more than about 50% longer in terms of time. The compile step is long - almost as long as a non-PGO test step - but then the PGO test step completes in half the time. The compilation bottleneck appears single-core limited. So not sure I'd want to double up all the builders, but having a main builder for PGO coverage seems feasible.

Oh, and for any builder definition, it looks like in addition to --pgo for the build step, we'll need +d on the test step so it uses a release python (python.exe not python_d.exe) as generated by the PGO build.

@zware zware transferred this issue from python/cpython Nov 22, 2024
@zware zware self-assigned this Nov 22, 2024
@zware
Copy link
Member

zware commented Nov 22, 2024

I've opened #561 for this. I'm trying something new in sharing this builder between both of our workers, @db3l. It's a hack, but hopefully it works out :)

@zware
Copy link
Member

zware commented Nov 23, 2024

@zware zware closed this as completed Nov 23, 2024
@mdboom
Copy link
Author

mdboom commented Nov 23, 2024

Thanks, all!

itamaro added a commit to itamaro/buildmaster-config that referenced this issue Nov 24, 2024
@itamaro
Copy link
Contributor

itamaro commented Nov 24, 2024

opened #565 to add a Windows PGO NoGIL builder

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants