Skip to content

make-symlinks-relative.sh: optimize - use multiple cores#411981

Merged
Mic92 merged 1 commit intoNixOS:stagingfrom
DavHau:symlinks-hook
Dec 4, 2025
Merged

make-symlinks-relative.sh: optimize - use multiple cores#411981
Mic92 merged 1 commit intoNixOS:stagingfrom
DavHau:symlinks-hook

Conversation

@DavHau
Copy link
Member

@DavHau DavHau commented May 29, 2025

This optimizes the performance of the hook on machines with multiple cores.

The previous implementation was slow, as it was launching two extra processes per symlink (readlink and ln) while not making use of multiprocessing.

The following improvements were made:

  • don't even execute the hook if dontRewriteSymlinks is set
  • replace multiple find calls with single find call over all outputs
  • use xargs -P to process symlinks in parallel
  • add test to check that the hook operates as expected

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • Nixpkgs 25.11 Release Notes (or backporting 24.11 and 25.05 Nixpkgs Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
  • NixOS 25.11 Release Notes (or backporting 24.11 and 25.05 NixOS Release notes)
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@nix-owners nix-owners bot requested a review from Ericson2314 May 29, 2025 06:42
@github-actions github-actions bot added 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. labels May 29, 2025
@DavHau
Copy link
Member Author

DavHau commented Jun 6, 2025

I also added more tests for parallelRun and parallelMap for the scenario where the input is empty (like when find doesn't find anything)

@DavHau DavHau requested a review from Mic92 June 6, 2025 15:53
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jun 16, 2025
@DavHau
Copy link
Member Author

DavHau commented Dec 3, 2025

Since parallelMap and parallelRun were reverted, I also reverted this optimization to the original implementation. Please have a look again @Mic92

@nixpkgs-ci nixpkgs-ci bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Dec 3, 2025
This optimizes the performance of the hook on machines with multiple cores.

The previous implementation was slow, as it was launching two extra processes per symlink (readlink and ln) while not making use of multiprocessing.

The following improvements were made:
- don't even execute the hook if dontRewriteSymlinks is set
- replace multiple find calls with single find call over all outputs
- use xargs -P to process symlinks in parallel
- add test to check that the hook operates as expected
@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux-stdenv This PR causes stdenv to rebuild on Linux and must target a staging branch. 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. labels Dec 3, 2025
@Mic92 Mic92 added this pull request to the merge queue Dec 4, 2025
Merged via the queue into NixOS:staging with commit c13a612 Dec 4, 2025
29 of 31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 10.rebuild-linux-stdenv This PR causes stdenv to rebuild on Linux and must target a staging branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants