Flake eval cache fixes - #15288
Merged
Merged
Conversation
github-actions
Bot
added
the
with-tests
edolstra
reviewed
Feb 19, 2026
| } else { | ||
| state->insertAttribute.use()(key.first)(symbols[key.second])(AttrType::FullAttrs) (0, false).exec(); | ||
| rowId = state->db.getLastInsertedRowId(); | ||
| } |
Member
There was a problem hiding this comment.
What's the performance impact of this (since it doubles the number of SQLite calls)?
Maybe you could use INSERT ... ON CONFLICT DO UPDATE to do it in a single SQLite call that should preserve the rowid (unlike INSERT OR REPLACE).
roberth
force-pushed
the
flake-eval-cache-fixes
branch
from
February 23, 2026 10:48
2af7f1e to
06b9384
Compare
Contributor
|
Rebase and merge? |
roberth
force-pushed
the
flake-eval-cache-fixes
branch
from
May 24, 2026 14:26
06b9384 to
36768cf
Compare
pkpbynum
approved these changes
Aug 14, 2026
pkpbynum
left a comment
pkpbynum
left a comment
Contributor
There was a problem hiding this comment.
👍 on this PR, was looking into nix develop cache misses just now and it's fixed precisely by f7d66e6
testing:
# Stable nix
% rm -rf ~/.cache/nix/eval-cache-v6
% nix --version
nix (Nix) 2.34.8
% nix develop -c true
nix develop -c true 1.86s user 0.67s system 68% cpu 3.684 total
% time nix develop -c true
nix develop -c true 1.36s user 0.55s system 62% cpu 3.039 total
% time nix develop -c true
nix develop -c true 0.17s user 0.19s system 44% cpu 0.809 total
# This branch
% rm -rf ~/.cache/nix/eval-cache-v6
% ./result/bin/nix --version
nix (Nix) 2.36.0pre20260720_dirty
% git diff
% time ./result/bin/nix develop -c true
./result/bin/nix develop -c true 2.20s user 0.82s system 69% cpu 4.359 total
% time ./result/bin/nix develop -c true
./result/bin/nix develop -c true 0.16s user 0.19s system 45% cpu 0.783 total
The in-memory cached value was created with an empty context `{}` instead
of copying the actual context from the value. This caused string contexts
to be lost even though they were correctly written to the database.
When setAttrs "seals" an attrset (transitions from Placeholder to FullAttrs), the previous INSERT OR REPLACE would delete and recreate the row with a new rowid. This orphaned any children cached under the old rowid, causing cache misses on subsequent builds. This explains the "third time's a charm" behavior: on the 2nd build, orphaned children are re-cached under the new parent rowid. With deeper nesting, even more evaluations could be required as each level of sealing orphans the next level's children. Fix by using INSERT ... ON CONFLICT DO UPDATE (upsert) to preserve the rowid, and INSERT OR IGNORE for children to preserve existing cached values. Also delete redundant Missing entries since we now know the complete set. This requires SQLite >= 3.35 for the RETURNING clause, now declared as an explicit dependency of libexpr.
Verify that evaluation caching works on the second build by checking that builtins.trace output only appears on the first evaluation.
edolstra
force-pushed
the
flake-eval-cache-fixes
branch
from
September 1, 2026 14:42
36768cf to
4d9b962
Compare
edolstra
approved these changes
Sep 1, 2026
Member
|
Sorry for not looking into this sooner. Thanks, I've resolved the merge conflict! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Get fixes into the release.
This fixes a problem where the third instead of second eval come from cache.
I haven't reproduced the string context splitting problem outside of a unit test in my WIP eval cache work - I would prefer to spend time improving that work than producing a test case for this unimportant corner case.
Context
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.