Skip to content
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

boot: use several processes to bootstrap on Win32 #9613

Closed
wants to merge 1 commit into from

Conversation

emillon
Copy link
Collaborator

@emillon emillon commented Jan 3, 2024

This workaround does not seem to be necessary (anymore?); the mechanism with several commands work, including when concurrency = 1.

This workaround does not seem to be necessary (anymore?); the mechanism
with several commands work, including when `concurrency = 1`.

Signed-off-by: Etienne Millon <[email protected]>
@emillon emillon force-pushed the boot-windows-parallel branch from 6c4c2ed to c510964 Compare January 3, 2024 16:38
@emillon emillon marked this pull request as ready for review January 3, 2024 16:40
@emillon
Copy link
Collaborator Author

emillon commented Jan 3, 2024

@dra27 you wrote this bootstrap code; do you remember why it was useful to have windows-specific behavior here?
(context is #9507 and we're wondering if we should remove that code path).

I'll also try to find a way to benchmark how much memory that saves.

@dra27
Copy link
Member

dra27 commented Jan 5, 2024

I have a vague recollection it was sufficiently faster to justify the extra code path

@dra27
Copy link
Member

dra27 commented Jan 5, 2024

Indeed - it seemed to be just empirical evidence (see #2854) - if it’s not the case any more, then marvellous!

@emillon
Copy link
Collaborator Author

emillon commented Jan 8, 2024

I ran the numbers:

OS strategy time
linux single 33.59
linux j1 41.11
linux j2 26.76
linux j3 21.36
linux j4 19.55
windows single 380.29
windows j1 302.25
windows j2 280.99
windows j3 323.05
windows j4 346.51
Bootstrap times for linux

image

Bootstrap times for windows

image

I agree that there's something weird going on with Windows, and it's probably pretty noisy (I only took one measurement here) but all in all it doesn't that the separate code path is worth it.

(I tried to benchmark memory usage, but did not fine any difference either using cygwin's /usr/bin/time nor with this approach using powershell).

@emillon
Copy link
Collaborator Author

emillon commented Jan 15, 2024

Closed in favor of #9735.

@emillon emillon closed this Jan 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants