Skip to content

Make GitFileSystemObjectSink multi-threaded#12087

Merged
edolstra merged 8 commits intoNixOS:masterfrom
DeterminateSystems:multithreaded-git-sink
Dec 19, 2025
Merged

Make GitFileSystemObjectSink multi-threaded#12087
edolstra merged 8 commits intoNixOS:masterfrom
DeterminateSystems:multithreaded-git-sink

Conversation

@edolstra
Copy link
Member

@edolstra edolstra commented Dec 19, 2024 •
edited
Loading

Motivation

This speeds up importing tarballs into the Git cache.

Time for importing a nixpkgs tarballs on Nix master:

# benchmark -t 25 -- bash -c 'rm -rf ~/.cache/nix/tarball-cache-v2/; ./result/bin/nix flake metadata tarball+file:///home/eelco/Dev/nix/nixpkgs.tar.gz'
...
maximum RSS:        median = 416220.0000  mean = 416847.4545  stddev =   2112.1267  min = 413384.0000  max = 421224.0000
soft page faults:   median = 151909.5000  mean = 151902.3636  stddev =   1450.3386  min = 147870.0000  max = 153540.0000
system CPU time:    median =      0.3226  mean =      0.3226  stddev =      0.0172  min =      0.2841  max =      0.3504
user CPU time:      median =     17.4927  mean =     17.4969  stddev =      0.1521  min =     17.2992  max =     17.8666
elapsed time:       median =      8.6704  mean =      8.6653  stddev =      0.1120  min =      8.5231  max =      8.9252

With this PR:

maximum RSS:        median = 584210.0000  mean = 583384.1818  stddev =  24970.3910  min = 532512.0000  max = 613760.0000
soft page faults:   median = 176620.0000  mean = 175756.7727  stddev =   4110.6419  min = 166738.0000  max = 180866.0000
system CPU time:    median =      0.5551  mean =      0.5538  stddev =      0.0247  min =      0.5085  max =      0.5988
user CPU time:      median =     15.8375  mean =     15.8021  stddev =      0.2406  min =     15.3682  max =     16.1459
elapsed time:       median =      2.8254  mean =      2.7995  stddev =      0.1647  min =      2.4551  max =      3.1430

I.e. a speedup from 8.6s to 2.8s.

Context

Add 👍 to pull requests you find important.

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

@github-actions github-actions bot added with-tests Issues related to testing. PRs with tests have some priority fetching Networking with the outside (non-Nix) world, input locking labels Dec 19, 2024
@edolstra edolstra force-pushed the multithreaded-git-sink branch from 4e96f92 to 4bf9371 Compare January 15, 2025 18:55
roberth
roberth reviewed Jan 24, 2025
View reviewed changes
@roberth

This comment was marked as outdated.

Sign in to view
@edolstra edolstra mentioned this pull request Jul 15, 2025
Merged
@edolstra edolstra force-pushed the multithreaded-git-sink branch from b23ef1b to 21a251b Compare December 16, 2025 14:56
@edolstra edolstra marked this pull request as ready for review December 16, 2025 14:58
@xokdvium
Copy link
Contributor

xokdvium commented Dec 16, 2025

Somehow managed to hit as assertion failure:

terminating due to unexpected unrecoverable internal error: Assertion 'totalBufSize == 0' failed in virtual nix::GitFileSystemObjectSinkImpl::~GitFileSystemObjectSinkImpl() at ../git-utils.cc:1099

Seems to happen when interrupting a fetch.

xokdvium
xokdvium reviewed Dec 16, 2025
View reviewed changes
@xokdvium

This comment was marked as resolved.

Sign in to view
xokdvium
xokdvium reviewed Dec 16, 2025
View reviewed changes
xokdvium
xokdvium reviewed Dec 16, 2025
View reviewed changes
xokdvium
xokdvium reviewed Dec 16, 2025
View reviewed changes
@edolstra edolstra force-pushed the multithreaded-git-sink branch from c8afca2 to 97e3816 Compare December 17, 2025 11:54
@edolstra
Copy link
Member Author

edolstra commented Dec 17, 2025

I've removed the assertion.

xokdvium
xokdvium requested changes Dec 17, 2025
View reviewed changes
Copy link
Contributor

@xokdvium xokdvium left a comment

Choose a reason for hiding this comment

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

Interrupting now also fails:

terminating due to unexpected unrecoverable internal error: Assertion 'child' failed in void nix::GitFileSystemObjectSinkImpl::addNode(State &, const CanonPath &, Child &&) at ../git-utils.cc:1157

xokdvium
xokdvium reviewed Dec 17, 2025
View reviewed changes
@edolstra edolstra added this pull request to the merge queue Dec 19, 2025
Merged via the queue into NixOS:master with commit 6b52fa8 Dec 19, 2025
16 checks passed
@edolstra edolstra deleted the multithreaded-git-sink branch December 19, 2025 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xokdvium xokdvium xokdvium approved these changes

+1 more reviewer

@roberth roberth roberth left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

fetching Networking with the outside (non-Nix) world, input locking with-tests Issues related to testing. PRs with tests have some priority

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@edolstra @roberth @xokdvium