From d5597b4784e04020b4ef4968a6887d4e22cc3edd Mon Sep 17 00:00:00 2001 From: Griffin Smith Date: Sun, 12 Jul 2020 17:29:45 -0400 Subject: feat(3p/nix): Statically pass bindings capacity where possible To aid in making the decision of where to (currently just statically) use a vector or btree as the backing implementation, add an extra constructor argument to Bindings::NewGC for a capacity, and use a (currently hardcoded at 32, for no good reason other than it felt like a reasonable number) pivot to switch between our possible backing implementations. Then, update all the call sites where it feels reasonable that we know the capacity statically to *pass* that capacity to the constructor. Paired-With: Luke Granger-Brown Paired-With: Vincent Ambo Paired-With: Perry Lorier Change-Id: I1858c161301a1cd0e83aeeb9a58839378869e71d Reviewed-on: https://cl.tvl.fyi/c/depot/+/1124 Tested-by: BuildkiteCI Reviewed-by: lukegb Reviewed-by: isomer --- third_party/nix/src/libexpr/attr-set.cc | 56 ++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 22 deletions(-) (limited to 'third_party/nix/src/libexpr/attr-set.cc') diff --git a/third_party/nix/src/libexpr/attr-set.cc b/third_party/nix/src/libexpr/attr-set.cc index 053d7d6f6c5e..0c8dc0baa42e 100644 --- a/third_party/nix/src/libexpr/attr-set.cc +++ b/third_party/nix/src/libexpr/attr-set.cc @@ -11,6 +11,8 @@ namespace nix { +constexpr size_t ATTRS_CAPACITY_PIVOT = 32; + BindingsIterator& BindingsIterator::operator++() { std::visit(util::overloaded{ [](AttributeMap::iterator& iter) { ++iter; }, @@ -132,27 +134,13 @@ void BTreeBindings::merge(Bindings& other) { } } -void EvalState::mkAttrs(Value& v, size_t capacity) { - clearValue(v); - v.type = tAttrs; - v.attrs = Bindings::NewGC(); - assert(v.attrs->begin() == v.attrs->begin()); - assert(v.attrs->end() == v.attrs->end()); - nrAttrsets++; - nrAttrsInAttrsets += capacity; -} - -/* Create a new attribute named 'name' on an existing attribute set stored - in 'vAttrs' and return the newly allocated Value which is associated with - this attribute. */ -Value* EvalState::allocAttr(Value& vAttrs, const Symbol& name) { - Value* v = allocValue(); - vAttrs.attrs->push_back(Attr(name, v)); - return v; -} - class VectorBindings : public Bindings { public: + VectorBindings() {}; + VectorBindings(size_t capacity) : attributes_() { + attributes_.reserve(capacity); + }; + size_t size() override; bool empty() override; void push_back(const Attr& attr) override; @@ -246,8 +234,32 @@ Bindings::iterator VectorBindings::end() { return BindingsIterator{attributes_.end()}; } -// TODO pick what to do based on size -Bindings* Bindings::NewGC() { return new (GC) BTreeBindings; } -// Bindings* Bindings::NewGC() { return new (GC) VectorBindings; } +Bindings* Bindings::NewGC(size_t capacity) { + if (capacity > ATTRS_CAPACITY_PIVOT) { + return new (GC) BTreeBindings; + } else { + return new (GC) VectorBindings(capacity); + } +} + +void EvalState::mkAttrs(Value& v, size_t capacity) { + clearValue(v); + v.type = tAttrs; + v.attrs = Bindings::NewGC(capacity); + assert(v.attrs->begin() == v.attrs->begin()); + assert(v.attrs->end() == v.attrs->end()); + nrAttrsets++; + nrAttrsInAttrsets += capacity; +} + +/* Create a new attribute named 'name' on an existing attribute set stored + in 'vAttrs' and return the newly allocated Value which is associated with + this attribute. */ +Value* EvalState::allocAttr(Value& vAttrs, const Symbol& name) { + Value* v = allocValue(); + vAttrs.attrs->push_back(Attr(name, v)); + return v; +} + } // namespace nix -- cgit 1.4.1