From ec2fd2dc23671463fa06222e60a383123380295b Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Mon, 27 Oct 2025 16:14:19 +0300 Subject: [PATCH] libexpr: Speed up BindingsBuilder::finishSizeIfNecessary Instead of iterating over the newly built bindings we can do a cheaper set_intersection to count duplicates or fall back to a per-element binary search over the "base" bindings. This speeds up `hello` evaluation by around 10ms (0.196s -> 0.187s) and `nixos.closures.ec2.x86_64-linux` by 140ms (2.744s -> 2.609s). This addresses a somewhat steep performance regression from 82315c3807f90be8f4728b32c343e6a2f31969e3 that reduced memory requirements of attribute set merges. With this patch we get back around to 2.31 level of eval performance while keeping the memory usage optimization. Also document the optimization a bit more. --- src/libexpr/include/nix/expr/attr-set.hh | 45 +++++++++++++++++++++--- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/src/libexpr/include/nix/expr/attr-set.hh b/src/libexpr/include/nix/expr/attr-set.hh index 46eecd9bd..f57302c42 100644 --- a/src/libexpr/include/nix/expr/attr-set.hh +++ b/src/libexpr/include/nix/expr/attr-set.hh @@ -5,6 +5,7 @@ #include "nix/expr/symbol-table.hh" #include +#include #include #include @@ -463,12 +464,48 @@ private: return bindings->baseLayer; } + /** + * If the bindings gets "layered" on top of another we need to recalculate + * the number of unique attributes in the chain. + * + * This is done by either iterating over the base "layer" and the newly added + * attributes and counting duplicates. If the base "layer" is big this approach + * is inefficient and we fall back to doing per-element binary search in the base + * "layer". + */ void finishSizeIfNecessary() { - if (hasBaseLayer()) - /* NOTE: Do not use std::ranges::distance, since Bindings is a sized - range, but we are calculating this size here. */ - bindings->numAttrsInChain = std::distance(bindings->begin(), bindings->end()); + if (!hasBaseLayer()) + return; + + auto & base = *bindings->baseLayer; + auto attrs = std::span(bindings->attrs, bindings->numAttrs); + + Bindings::size_type duplicates = 0; + + /* If the base bindings is smaller than the newly added attributes + iterate using std::set_intersection to run in O(|base| + |attrs|) = + O(|attrs|). Otherwise use an O(|attrs| * log(|base|)) per-attr binary + search to check for duplicates. Note that if we are in this code path then + |attrs| <= bindingsUpdateLayerRhsSizeThreshold, which 16 by default. We are + optimizing for the case when a small attribute set gets "layered" on top of + a much larger one. When attrsets are already small it's fine to do a linear + scan, but we should avoid expensive iterations over large "base" attrsets. */ + if (attrs.size() > base.size()) { + std::set_intersection( + base.begin(), + base.end(), + attrs.begin(), + attrs.end(), + boost::make_function_output_iterator([&]([[maybe_unused]] auto && _) { ++duplicates; })); + } else { + for (const auto & attr : attrs) { + if (base.get(attr.name)) + ++duplicates; + } + } + + bindings->numAttrsInChain = base.numAttrsInChain + attrs.size() - duplicates; } public: