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

refactor: nix: avoid adding search path symlinks to the nix store #2568

Closed
wants to merge 2 commits into from

Conversation

nomeata
Copy link
Contributor

@nomeata nomeata commented Sep 21, 2023

This fixes #2508.

(Includes release branch commits for easier testing, will fix that
before undrafting)

nomeata added a commit to nomeata/loogle that referenced this pull request Sep 21, 2023
@nomeata nomeata changed the title nix: Avoid adding search path symlinks to the nix store chore(nix): Avoid adding search path symlinks to the nix store Sep 21, 2023
@nomeata nomeata changed the title chore(nix): Avoid adding search path symlinks to the nix store refactor(nix): Avoid adding search path symlinks to the nix store Sep 21, 2023
@Kha
Copy link
Member

Kha commented Sep 21, 2023

Nice, any benchmarks yet? E.g. echo "-- $(date)" >> src/Init/Prelude.lean; time nix build --offline

@nomeata
Copy link
Contributor Author

nomeata commented Sep 21, 2023

Before:

real	8m9,954s
user	0m1,514s
sys	0m0,785s

after

real	8m56,574s
user	0m1,453s
sys	0m0,760s

I wonder if it is even significant, -j8 on my laptop. Let me rerun… nope, seems reproducible.

I didn’t expect a regression, it has to create the same symlink trees as before, just during the per-module build rather than as a separate nix derivation. So that’s strange.

Maybe invoking cp more often makes a difference?

(Of course, the goal isn’t to make it faster per se, it’s to not have these symlink trees in /nix/store because they eat inodes like hell… but it’s worrying that it did become slower.)

nomeata added a commit to nomeata/loogle that referenced this pull request Sep 21, 2023
@nomeata
Copy link
Contributor Author

nomeata commented Sep 21, 2023

Still no good idea why it’s slower, but I can report that the code works building all of mathlib (as part of the loogle deploy)

@Kha
Copy link
Member

Kha commented Sep 22, 2023

Maybe invoking cp more often makes a difference?

Could be. Try measuring your script vs the corresponding lndir?

@nomeata nomeata changed the base branch from master to releases/v4.1.0 September 23, 2023 08:19
@nomeata nomeata changed the base branch from releases/v4.1.0 to master September 23, 2023 08:20
@nomeata
Copy link
Contributor Author

nomeata commented Sep 23, 2023

Any idea why CI doesn’t run?

@Kha
Copy link
Member

Kha commented Sep 23, 2023

Not sure. You might need to push again after the base change.

@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label Sep 23, 2023
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Sep 23, 2023
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the builds-mathlib CI has verified that Mathlib builds against this PR label Sep 23, 2023
@leanprover-community-mathlib4-bot
Copy link
Collaborator

@nomeata nomeata changed the title refactor(nix): Avoid adding search path symlinks to the nix store refactor: nix: avoid adding search path symlinks to the nix store Dec 12, 2023
@nomeata
Copy link
Contributor Author

nomeata commented Jan 25, 2024

No longer using this for loogle, so closing. Hopefully we will have a good lake-based nix setup eventually.

@nomeata nomeata closed this Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builds-mathlib CI has verified that Mathlib builds against this PR toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

nix: nixBuildPackage’s depRoots are costly on the store
3 participants