Skip to content

nix nar {ls,cat}: Optimize, make nix nar cat work on pipes too#14732

Merged
Ericson2314 merged 3 commits intomasterfrom
optimize-nar-cat
Dec 8, 2025
Merged

nix nar {ls,cat}: Optimize, make nix nar cat work on pipes too#14732
Ericson2314 merged 3 commits intomasterfrom
optimize-nar-cat

Conversation

@xokdvium
Copy link
Contributor

@xokdvium xokdvium commented Dec 8, 2025

Motivation

This (nix nar cat working with pipes) was lost after 2.32 while making the accessor lazy. We can restore the support for it pretty easily. Also this is significant optimization for nix nar cat.
E.g. with a NAR of a linux repo this speeds up by ~3x:

Benchmark 1: nix nar cat /tmp/linux.nar README
  Time (mean ± σ):     737.2 ms ±   5.6 ms    [User: 298.1 ms, System: 435.7 ms]
  Range (min … max):   728.6 ms … 746.9 ms    10 runs

Benchmark 2: build/src/nix/nix nar cat /tmp/linux.nar README
  Time (mean ± σ):     253.5 ms ±   2.9 ms    [User: 56.4 ms, System: 196.3 ms]
  Range (min … max):   248.1 ms … 258.7 ms    12 runs

Context

#14273

cc @myclevorname, you might be interested in this. Do note that we still have to parse the full NAR to validate it fully. Cutting off early wouldn't be correct.


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

By default windows doesn't allow inheriting handles anyway. These comments
are just confusing at this point.
@xokdvium xokdvium requested a review from edolstra as a code owner December 8, 2025 00:00
@github-actions github-actions bot added new-cli Relating to the "nix" command with-tests Issues related to testing. PRs with tests have some priority labels Dec 8, 2025
The whole NarAccessor -> listing -> lazy NarAccessor is very weird. Source
can now be seek-ed over when supported, so we can support it pretty easily.
Alternatively we could also make it single-pass very easily with a custom
FileSystemObjectSink. It will get removed in a follow-up commit anyway.
This was lost after 2.32 while making the accessor lazy. We can restore the support
for it pretty easily. Also this is significant optimization for nix nar cat.
E.g. with a NAR of a linux repo this speeds up by ~3x:

Benchmark 1: nix nar cat /tmp/linux.nar README
  Time (mean ± σ):     737.2 ms ±   5.6 ms    [User: 298.1 ms, System: 435.7 ms]
  Range (min … max):   728.6 ms … 746.9 ms    10 runs

Benchmark 2: build/src/nix/nix nar cat /tmp/linux.nar README
  Time (mean ± σ):     253.5 ms ±   2.9 ms    [User: 56.4 ms, System: 196.3 ms]
  Range (min … max):   248.1 ms … 258.7 ms    12 runs

GetNarBytes seekableGetNarBytes(Descriptor fd);

ref<SourceAccessor> makeLazyNarAccessor(const nlohmann::json & listing, GetNarBytes getNarBytes);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW this should use the new listing types now that we have them (I missed them before)

@Ericson2314 Ericson2314 added this pull request to the merge queue Dec 8, 2025
Merged via the queue into master with commit ffc5dff Dec 8, 2025
20 checks passed
@Ericson2314 Ericson2314 deleted the optimize-nar-cat branch December 8, 2025 06:49
@edolstra edolstra mentioned this pull request Dec 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new-cli Relating to the "nix" command with-tests Issues related to testing. PRs with tests have some priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants