libstore: avoid PathSet allocation in derivation unparse

[Zaczero]

• Derivation::unparse() built a temporary PathSet via printStorePathSet(inputSrcs) only to immediately serialize it, which was already flagged as // FIXME: slow.

• This reduces allocations/copies on a hot path during derivation (un)parsing; in local microbenchmarks (BM_UnparseRealDerivationFile/hello) it improves runtime by ~8%.

Motivation

Context

---

Add 👍 to pull requests you find important.

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

[xokdvium]

SGTM. I suppose we also skip the potential string escaping because derivation names cannot contain any characters that need escapes, right?

[Zaczero]

@xokdvium I don't think the original was escaping anything (I didn't review whether it's correct or not, I assume it is). It did build a set of strings and then used printUnquotedStrings to print each as-is. The new approach simply iterates and prints directly without allocations in this hot path.

[Ericson2314]

See also my review in #14825 (review)

Here's what I would like

1. Move concatStrings from util.hh to strings.hh. That is my mistake not doing that when I broke up most of util.hh. (Feel free to move some other things there while you are at it.

2. Just as we have basicSplitStringInto vs basicSplitString in strings-inline.hh, we should have a version of concatStrings (and the underlying concatStringSep) that appends to an existing string.

3. I think you can just change printStorePathSet to do the appending way, and that will benefit most call sites, and for the remainder you can either just make the initial empty string manually, or make a small overload wrapper that does it.

How does that sound? That allows us to have or cake (perf) and eat it (code reuse) too.

[Zaczero]

See also my review in #14825 (review)

Here's what I would like

1. Move concatStrings from util.hh to strings.hh. That is my mistake not doing that when I broke up most of util.hh. (Feel free to move some other things there while you are at it.
2. Just as we have basicSplitStringInto vs basicSplitString in strings-inline.hh, we should have a version of concatStrings (and the underlying concatStringSep) that appends to an existing string.
3. I think you can just change printStorePathSet to do the appending way, and that will benefit most call sites, and for the remainder you can either just make the initial empty string manually, or make a small overload wrapper that does it.

How does that sound? That allows us to have or cake (perf) and eat it (code reuse) too.

Sounds good, I'll dig into it.