Skip to content

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Jul 31, 2025

Motivation

There's some low hanging fruit in terms of code simplification and getting rid
of unnecessary copying (see the second commit). Also moving the cache entry to the back of the lru list can be
made much cheaper by just using splice method, which just reassigns some pointers in the linked list (having pointer stability is the main purpose of std::list after all).

Context

Some unified caching primitives are useful to reuse in other places (e.g. #13137 (comment))


Add 👍 to pull requests you find important.

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


This is an automatic backport of pull request #13193 done by [Mergify](https://mergify.com).

xokdvium added 4 commits July 31, 2025 13:25
Rip off the band-aid for further refactors. The diff is
very small, so it makes to get it out of the way first.

(cherry picked from commit 90d70aa)

# Conflicts:
#	maintainers/flake-module.nix
This gets rid of some ugly std::string_view -> std::string
conversions, which are an eye-sore and lead to extra copying.

(cherry picked from commit cd61e92)

# Conflicts:
#	src/libstore/store-api.cc
Splicing the list element to the back can be done in
a much simpler and concise way without the need for
erasing and re-inserting the element. Doing it this
way is equivalent to just moving node pointers around,
whereas inserting/erasing allocates/deallocates new nodes.

(cherry picked from commit 2f2e041)
The underlying containers are already noexcept to destroy and dtors are
noexcept in general.

(cherry picked from commit d955b40)
@mergify mergify bot requested a review from Ericson2314 as a code owner July 31, 2025 13:25
@mergify mergify bot added the automatic backport This PR is a backport produced by automation (does not trigger backporting) label Jul 31, 2025
@mergify mergify bot requested a review from edolstra as a code owner July 31, 2025 13:25
Copy link
Contributor Author

mergify bot commented Jul 31, 2025

Cherry-pick of 90d70aa has failed:

On branch mergify/bp/2.28-maintenance/pr-13193
Your branch is up to date with 'origin/2.28-maintenance'.

You are currently cherry-picking commit 90d70aa4c.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   maintainers/flake-module.nix

no changes added to commit (use "git add" and/or "git commit -a")

Cherry-pick of cd61e92 has failed:

On branch mergify/bp/2.28-maintenance/pr-13193
Your branch is ahead of 'origin/2.28-maintenance' by 1 commit.
  (use "git push" to publish your local commits)

You are currently cherry-picking commit cd61e922f.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   src/libutil/include/nix/util/lru-cache.hh

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   src/libstore/store-api.cc

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@mergify mergify bot added conflicts automatic backport This PR is a backport produced by automation (does not trigger backporting) labels Jul 31, 2025
@github-actions github-actions bot added the store Issues and pull requests concerning the Nix store label Jul 31, 2025
@edolstra edolstra closed this Aug 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automatic backport This PR is a backport produced by automation (does not trigger backporting) conflicts merge-queue store Issues and pull requests concerning the Nix store
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants