Skip to content

optimise: reduce lstat and use correct inode#15831

Open
tomberek wants to merge 1 commit into
NixOS:masterfrom
tomberek:optimise-reduce-lstat
Open

optimise: reduce lstat and use correct inode#15831
tomberek wants to merge 1 commit into
NixOS:masterfrom
tomberek:optimise-reduce-lstat

Conversation

@tomberek
Copy link
Copy Markdown
Contributor

Motivation

Make nix store optimise faster. Noticed 4 lstat's taking place per operation in an strace. These come from lstate and pathExists calls.

Claude helped. Also found/fixed the issue of saving the inode of the link, rather than the original file.

@tomberek tomberek requested a review from Ericson2314 as a code owner May 11, 2026 08:34
@github-actions github-actions Bot added the store Issues and pull requests concerning the Nix store label May 11, 2026
@xokdvium
Copy link
Copy Markdown
Contributor

Also found/fixed the issue of saving the inode of the link, rather than the original file.

Why does it matter? It's the same inode either way.

@tomberek
Copy link
Copy Markdown
Contributor Author

tomberek commented May 11, 2026

In case of a race that another optimise got there first. I guess it would be rare. (i was thinking multiple could run at once)

Remove that change to make it minimal? (and remove that extra lstat)

Before this change, optimisePath_ would lstat the .links entry up to 4
times per file:
- pathExists(linkPath) does an lstat
- lstat(linkPath) to check for corruption
- pathExists(linkPath) again to check if link needs creation
- lstat(linkPath) again after the checks

This changes the code to lstat once via maybeLstat() and reuse the
result throughout the function, reducing syscalls by 50-75% during
optimization.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@tomberek tomberek force-pushed the optimise-reduce-lstat branch from 5094a61 to 59d12ee Compare May 12, 2026 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

store Issues and pull requests concerning the Nix store

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants