-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
[illumos] attempt to use posix_spawn to spawn processes #137463
Conversation
illumos has `posix_spawn`, and the very newest versions also have `_addchdir`, so use that. This is a nice ~4x performance improvement for process creation. My go-to as usual is nextest against the clap repo, which acts as a stress test for process creation -- with [this commit]: ```console $ cargo nextest run -E 'not test(ui_tests) and not test(example_tests)' before: Summary [ 1.747s] 879 tests run: 879 passed, 2 skipped after: Summary [ 0.445s] 879 tests run: 879 passed, 2 skipped ``` [this commit]: clap-rs/clap@fde45f9
rustbot has assigned @Mark-Simulacrum. Use |
These commits modify the If this was unintentional then you should revert the changes before this PR is merged. |
@bors r+ |
…Mark-Simulacrum [illumos] attempt to use posix_spawn to spawn processes illumos has `posix_spawn`, and the very newest versions also have `_addchdir`, so use that. POSIX standardized this function so I also added a weak symbol lookup for the non `_np` version. (illumos has both.) This probably also works on Solaris, but I don't have access to an installation to validate this so I decided to focus on illumos instead. This is a nice ~4x performance improvement for process creation. My go-to as usual is nextest against the clap repo, which acts as a stress test for process creation -- with [this commit]: ```console $ cargo nextest run -E 'not test(ui_tests) and not test(example_tests)' before: Summary [ 1.747s] 879 tests run: 879 passed, 2 skipped after: Summary [ 0.445s] 879 tests run: 879 passed, 2 skipped ``` [this commit]: clap-rs/clap@fde45f9
…Mark-Simulacrum [illumos] attempt to use posix_spawn to spawn processes illumos has `posix_spawn`, and the very newest versions also have `_addchdir`, so use that. POSIX standardized this function so I also added a weak symbol lookup for the non `_np` version. (illumos has both.) This probably also works on Solaris, but I don't have access to an installation to validate this so I decided to focus on illumos instead. This is a nice ~4x performance improvement for process creation. My go-to as usual is nextest against the clap repo, which acts as a stress test for process creation -- with [this commit]: ```console $ cargo nextest run -E 'not test(ui_tests) and not test(example_tests)' before: Summary [ 1.747s] 879 tests run: 879 passed, 2 skipped after: Summary [ 0.445s] 879 tests run: 879 passed, 2 skipped ``` [this commit]: clap-rs/clap@fde45f9
…iaskrgr Rollup of 12 pull requests Successful merges: - rust-lang#136975 (Look for `python3` first on MacOS, not `py`) - rust-lang#137240 (Slightly reformat `std::fs::remove_dir_all` error docs) - rust-lang#137303 (Remove `MaybeForgetReturn` suggestion) - rust-lang#137463 ([illumos] attempt to use posix_spawn to spawn processes) - rust-lang#137722 (`librustdoc`: 2024 edition! 🎊) - rust-lang#137758 (fix usage of ty decl macro fragments in attributes) - rust-lang#137772 (Fix char count in `Display` for `ByteStr`) - rust-lang#137805 (adjust Layout debug printing to match the internal field name) - rust-lang#137808 (Do not require that unsafe fields lack drop glue) - rust-lang#137913 (Allow struct field default values to reference struct's generics) - rust-lang#137963 (Add ``dyn`` keyword to `E0373` examples) - rust-lang#137975 (Remove unused `PpMode::needs_hir`) r? `@ghost` `@rustbot` modify labels: rollup
…Mark-Simulacrum [illumos] attempt to use posix_spawn to spawn processes illumos has `posix_spawn`, and the very newest versions also have `_addchdir`, so use that. POSIX standardized this function so I also added a weak symbol lookup for the non `_np` version. (illumos has both.) This probably also works on Solaris, but I don't have access to an installation to validate this so I decided to focus on illumos instead. This is a nice ~4x performance improvement for process creation. My go-to as usual is nextest against the clap repo, which acts as a stress test for process creation -- with [this commit]: ```console $ cargo nextest run -E 'not test(ui_tests) and not test(example_tests)' before: Summary [ 1.747s] 879 tests run: 879 passed, 2 skipped after: Summary [ 0.445s] 879 tests run: 879 passed, 2 skipped ``` [this commit]: clap-rs/clap@fde45f9
Rollup of 13 pull requests Successful merges: - rust-lang#136581 (Retire the legacy `Makefile`-based `run-make` test infra) - rust-lang#136865 (Perform deeper compiletest path normalization for `$TEST_BUILD_DIR` to account for compare-mode/debugger cases, and normalize long type file filename hashes) - rust-lang#137147 (Add exclude to config.toml) - rust-lang#137327 (Undeprecate env::home_dir) - rust-lang#137463 ([illumos] attempt to use posix_spawn to spawn processes) - rust-lang#137477 (uefi: Add Service Binding Protocol abstraction) - rust-lang#137502 (Don't include global asm in `mir_keys`, fix error body synthesis) - rust-lang#137534 ([rustdoc] hide item that is not marked as doc(inline) and whose src is doc(hidden)) - rust-lang#137565 (Try to point of macro expansion from resolver and method errors if it involves macro var) - rust-lang#137643 (Add DWARF test case for non-C-like `repr128` enums) - rust-lang#137722 (`librustdoc`: 2024 edition! 🎊) - rust-lang#137836 (Set `target_vendor = "openwrt"` on `mips64-openwrt-linux-musl`) - rust-lang#137949 (Update MSVC INSTALL.md instructions to recommend VS 2022 + recent Windows 10/11 SDK) Failed merges: - rust-lang#137798 (ci: use ubuntu 24 on arm large runner) r? `@ghost` `@rustbot` modify labels: rollup
Cc @psumbera for Solaris |
…Mark-Simulacrum [illumos] attempt to use posix_spawn to spawn processes illumos has `posix_spawn`, and the very newest versions also have `_addchdir`, so use that. POSIX standardized this function so I also added a weak symbol lookup for the non `_np` version. (illumos has both.) This probably also works on Solaris, but I don't have access to an installation to validate this so I decided to focus on illumos instead. This is a nice ~4x performance improvement for process creation. My go-to as usual is nextest against the clap repo, which acts as a stress test for process creation -- with [this commit]: ```console $ cargo nextest run -E 'not test(ui_tests) and not test(example_tests)' before: Summary [ 1.747s] 879 tests run: 879 passed, 2 skipped after: Summary [ 0.445s] 879 tests run: 879 passed, 2 skipped ``` [this commit]: clap-rs/clap@fde45f9
Rollup of 10 pull requests Successful merges: - rust-lang#134063 (dec2flt: Clean up float parsing modules) - rust-lang#136581 (Retire the legacy `Makefile`-based `run-make` test infra) - rust-lang#136975 (Look for `python3` first on MacOS, not `py`) - rust-lang#137147 (Add exclude to config.toml) - rust-lang#137240 (Slightly reformat `std::fs::remove_dir_all` error docs) - rust-lang#137303 (Remove `MaybeForgetReturn` suggestion) - rust-lang#137327 (Undeprecate env::home_dir) - rust-lang#137463 ([illumos] attempt to use posix_spawn to spawn processes) - rust-lang#137534 ([rustdoc] hide item that is not marked as doc(inline) and whose src is doc(hidden)) - rust-lang#137829 (Stabilize [T]::split_off... methods) r? `@ghost` `@rustbot` modify labels: rollup
Thank you for adding me on CC. @sunshowers any chance to include also |
Thanks! Could you also validate the patch's behavior? Something like nextest might work, or some tooling you have that spawns processes in complex ways. |
…Mark-Simulacrum [illumos] attempt to use posix_spawn to spawn processes illumos has `posix_spawn`, and the very newest versions also have `_addchdir`, so use that. POSIX standardized this function so I also added a weak symbol lookup for the non `_np` version. (illumos has both.) This probably also works on Solaris, but I don't have access to an installation to validate this so I decided to focus on illumos instead. This is a nice ~4x performance improvement for process creation. My go-to as usual is nextest against the clap repo, which acts as a stress test for process creation -- with [this commit]: ```console $ cargo nextest run -E 'not test(ui_tests) and not test(example_tests)' before: Summary [ 1.747s] 879 tests run: 879 passed, 2 skipped after: Summary [ 0.445s] 879 tests run: 879 passed, 2 skipped ``` [this commit]: clap-rs/clap@fde45f9
Rollup of 10 pull requests Successful merges: - rust-lang#134063 (dec2flt: Clean up float parsing modules) - rust-lang#136662 (Count char width at most once in `Formatter::pad`) - rust-lang#137011 (Promote ohos targets to tier2 with host tools.) - rust-lang#137077 (Postprocess bootstrap metrics into GitHub job summary) - rust-lang#137327 (Undeprecate env::home_dir) - rust-lang#137373 (Compile run-make-support and run-make tests with the bootstrap compiler) - rust-lang#137463 ([illumos] attempt to use posix_spawn to spawn processes) - rust-lang#137477 (uefi: Add Service Binding Protocol abstraction) - rust-lang#137569 (Stablize `string_extend_from_within`) - rust-lang#137667 (Add `dist::Gcc` build step) r? `@ghost` `@rustbot` modify labels: rollup
I'm afraid I won't be able to test it quickly. I mean I don't really have working test environment (and the knowledge) and don't have any other test in this area. In that case I don't want to block you with the ingreation. So I think you can go ahead. I can add it on my todo list for future work then. Thank you! |
Thanks -- just a bit worried about causing unexpected regressions. For the record, whenever you get the time to test this, this is the process I followed: Building Rust:
Building nextest with the patched toolchain:
Checking out clap:
Hope this helps. |
…Mark-Simulacrum [illumos] attempt to use posix_spawn to spawn processes illumos has `posix_spawn`, and the very newest versions also have `_addchdir`, so use that. POSIX standardized this function so I also added a weak symbol lookup for the non `_np` version. (illumos has both.) This probably also works on Solaris, but I don't have access to an installation to validate this so I decided to focus on illumos instead. This is a nice ~4x performance improvement for process creation. My go-to as usual is nextest against the clap repo, which acts as a stress test for process creation -- with [this commit]: ```console $ cargo nextest run -E 'not test(ui_tests) and not test(example_tests)' before: Summary [ 1.747s] 879 tests run: 879 passed, 2 skipped after: Summary [ 0.445s] 879 tests run: 879 passed, 2 skipped ``` [this commit]: clap-rs/clap@fde45f9
…kingjubilee Rollup of 25 pull requests Successful merges: - rust-lang#134063 (dec2flt: Clean up float parsing modules) - rust-lang#136581 (Retire the legacy `Makefile`-based `run-make` test infra) - rust-lang#136662 (Count char width at most once in `Formatter::pad`) - rust-lang#136798 (Added documentation for flushing per rust-lang#74348) - rust-lang#137240 (Slightly reformat `std::fs::remove_dir_all` error docs) - rust-lang#137303 (Remove `MaybeForgetReturn` suggestion) - rust-lang#137327 (Undeprecate env::home_dir) - rust-lang#137463 ([illumos] attempt to use posix_spawn to spawn processes) - rust-lang#137477 (uefi: Add Service Binding Protocol abstraction) - rust-lang#137565 (Try to point of macro expansion from resolver and method errors if it involves macro var) - rust-lang#137569 (Stabilize `string_extend_from_within`) - rust-lang#137612 (Update bootstrap to edition 2024) - rust-lang#137633 (Only use implied bounds hack if bevy, and use deeply normalize in implied bounds hack) - rust-lang#137643 (Add DWARF test case for non-C-like `repr128` enums) - rust-lang#137679 (Various coretests improvements) - rust-lang#137723 (Make `rust.description` more general-purpose and pass `CFG_VER_DESCRIPTION`) - rust-lang#137758 (fix usage of ty decl macro fragments in attributes) - rust-lang#137764 (Ensure that negative auto impls are always applicable) - rust-lang#137772 (Fix char count in `Display` for `ByteStr`) - rust-lang#137798 (ci: use ubuntu 24 on arm large runner) - rust-lang#137805 (adjust Layout debug printing to match the internal field name) - rust-lang#137808 (Do not require that unsafe fields lack drop glue) - rust-lang#137820 (Clarify why InhabitedPredicate::instantiate_opt exists) - rust-lang#137825 (Provide more context on resolve error caused from incorrect RTN) - rust-lang#138028 (compiler: add `ExternAbi::is_rustic_abi`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 20 pull requests Successful merges: - rust-lang#134063 (dec2flt: Clean up float parsing modules) - rust-lang#136581 (Retire the legacy `Makefile`-based `run-make` test infra) - rust-lang#136662 (Count char width at most once in `Formatter::pad`) - rust-lang#136764 (Make `ptr_cast_add_auto_to_object` lint into hard error) - rust-lang#136798 (Added documentation for flushing per rust-lang#74348) - rust-lang#136865 (Perform deeper compiletest path normalization for `$TEST_BUILD_DIR` to account for compare-mode/debugger cases, and normalize long type file filename hashes) - rust-lang#136975 (Look for `python3` first on MacOS, not `py`) - rust-lang#136977 (Upload Datadog metrics with citool) - rust-lang#137240 (Slightly reformat `std::fs::remove_dir_all` error docs) - rust-lang#137298 (Check signature WF when lowering MIR body) - rust-lang#137463 ([illumos] attempt to use posix_spawn to spawn processes) - rust-lang#137477 (uefi: Add Service Binding Protocol abstraction) - rust-lang#137569 (Stabilize `string_extend_from_within`) - rust-lang#137633 (Only use implied bounds hack if bevy, and use deeply normalize in implied bounds hack) - rust-lang#137679 (Various coretests improvements) - rust-lang#137723 (Make `rust.description` more general-purpose and pass `CFG_VER_DESCRIPTION`) - rust-lang#137728 (Remove unsizing coercions for tuples) - rust-lang#137731 (Resume one waiter at once in deadlock handler) - rust-lang#137875 (mir_build: Integrate "simplification" steps into match-pair-tree creation) - rust-lang#138028 (compiler: add `ExternAbi::is_rustic_abi`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#137463 - sunshowers:illumos-posix-spawn, r=Mark-Simulacrum [illumos] attempt to use posix_spawn to spawn processes illumos has `posix_spawn`, and the very newest versions also have `_addchdir`, so use that. POSIX standardized this function so I also added a weak symbol lookup for the non `_np` version. (illumos has both.) This probably also works on Solaris, but I don't have access to an installation to validate this so I decided to focus on illumos instead. This is a nice ~4x performance improvement for process creation. My go-to as usual is nextest against the clap repo, which acts as a stress test for process creation -- with [this commit]: ```console $ cargo nextest run -E 'not test(ui_tests) and not test(example_tests)' before: Summary [ 1.747s] 879 tests run: 879 passed, 2 skipped after: Summary [ 0.445s] 879 tests run: 879 passed, 2 skipped ``` [this commit]: clap-rs/clap@fde45f9
illumos has
posix_spawn
, and the very newest versions also have_addchdir
, so use that. POSIX standardized this function so I also added a weak symbol lookup for the non_np
version. (illumos has both.)This probably also works on Solaris, but I don't have access to an installation to validate this so I decided to focus on illumos instead.
This is a nice ~4x performance improvement for process creation. My go-to as usual is nextest against the clap repo, which acts as a stress test for process creation -- with this commit: