Skip to content

speedup derivation parsing by optimizing parseString#13694

Merged
Mic92 merged 1 commit intoNixOS:masterfrom
NaN-git:opt-parseString
Aug 6, 2025
Merged

speedup derivation parsing by optimizing parseString#13694
Mic92 merged 1 commit intoNixOS:masterfrom
NaN-git:opt-parseString

Conversation

@NaN-git
Copy link
Contributor

@NaN-git NaN-git commented Aug 5, 2025

Motivation

According to #13569 parseString is quite slow because it uses two loops scanning the input string byte-by-byte.

Replacing the loops by calls to find yields the main speedup of this PR.

Context

Even if the loops would be auto-vectorization friendly, auto-vectorization wouldn't be very effective because Nix is compiled for generic CPU targets and cannot use advanced instruction sets like AVX2 or AVX-512.
Standard libraries can implement variants for different instruction sets and can dispatch them dynamically. At least some generic, parallelized implementations of the most common string functions should be available.

With the PR the hello derivation is parsed ~7% faster, while the speedup of the firefox derivation benchmark is ~20% measured on Zen3.

The actual bottleneck in the firefox derivation benchmark isn't parseString anymore, but rather the JSON parsing to structuredAttrs. Without actually parsing the JSON the throughput increases from ~117 MB/s to ~380 MB/s.
I'm not sure whether this code path is still relevant.


Add 👍 to pull requests you find important.

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

@NaN-git NaN-git requested a review from Ericson2314 as a code owner August 5, 2025 21:33
@xokdvium
Copy link
Contributor

xokdvium commented Aug 5, 2025

Without actually parsing the JSON the throughput increases from ~117 MB/s to ~380 MB/s.

@Ericson2314, maybe doing the lazy parsing thing would be worthwhile actually?

@Mic92 Mic92 requested a review from Copilot August 6, 2025 10:47

This comment was marked as outdated.

@Mic92 Mic92 requested a review from Copilot August 6, 2025 14:38

This comment was marked as resolved.

@Mic92
Copy link
Member

Mic92 commented Aug 6, 2025

Without actually parsing the JSON the throughput increases from ~117 MB/s to ~380 MB/s.

@Ericson2314, maybe doing the lazy parsing thing would be worthwhile actually?

I did this here and here is the performance diff:

# After
BM_ParseRealDerivationFile/hello        13473 ns        13260 ns        52542 bytes_per_second=126.873Mi/s
BM_ParseRealDerivationFile/firefox      46698 ns        46619 ns        15576 bytes_per_second=327.842Mi/s

# Before
BM_ParseRealDerivationFile/hello        14720 ns        14505 ns        47658 bytes_per_second=115.98Mi/s
BM_ParseRealDerivationFile/firefox     266988 ns       266251 ns         2608 bytes_per_second=57.4028Mi/s

Just unsure how relevant this is. This would need some benchmarks where we actually parse derivations.

@Ericson2314
Copy link
Member

Yes, my take-away from the last time is while the JSON parsing is slow, derivation A-Term parsing performance also just doesn't matter than much, because GC doesn't (and shouldn't in the CA unstable feature case) need to parse drv files.

I think that means we can still do this PR, but it should be as much about code clarity as perform.

@NaN-git
Copy link
Contributor Author

NaN-git commented Aug 6, 2025

@Ericson2314 Are sure that this code path is used only during GC? Store::readDerivation uses this code path and this function seems to have many callers. Or isn't the JSON parsed then?

I think that @Mic92's deferred computation of structuredAttrs would speed up many paths.

@Mic92
Copy link
Member

Mic92 commented Aug 6, 2025

@Ericson2314 Are sure that the this code path is used only during GC? Store::readDerivation uses this code path and this function seems to have many callers. Or isn't the JSON parsed then?

I think that @Mic92's deferred computation of structuredAttrs would speed up many paths.

We should than see a speed up when evaluating nixpkgs?

@Mic92 Mic92 merged commit c76222e into NixOS:master Aug 6, 2025
14 checks passed
@Mic92
Copy link
Member

Mic92 commented Aug 6, 2025

@Ericson2314 Are sure that this code path is used only during GC? Store::readDerivation uses this code path and this function seems to have many callers. Or isn't the JSON parsed then?

I think that @Mic92's deferred computation of structuredAttrs would speed up many paths.

I couldn't find anything for the evaluation actually

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants