Merge pull request #5865 from pennae/memory-friendliness

be more memory friendly

src/libexpr/eval-inline.hh

@@ -24,6 +24,81 @@ LocalNoInlineNoReturn(void throwTypeError(const Pos & pos, const char * s, const
 }
 
 
+/* Note: Various places expect the allocated memory to be zeroed. */
+[[gnu::always_inline]]
+inline void * allocBytes(size_t n)
+{
+    void * p;
+#if HAVE_BOEHMGC
+    p = GC_MALLOC(n);
+#else
+    p = calloc(n, 1);
+#endif
+    if (!p) throw std::bad_alloc();
+    return p;
+}
+
+
+[[gnu::always_inline]]
+Value * EvalState::allocValue()
+{
+#if HAVE_BOEHMGC
+    /* We use the boehm batch allocator to speed up allocations of Values (of which there are many).
+       GC_malloc_many returns a linked list of objects of the given size, where the first word
+       of each object is also the pointer to the next object in the list. This also means that we
+       have to explicitly clear the first word of every object we take. */
+    if (!*valueAllocCache) {
+        *valueAllocCache = GC_malloc_many(sizeof(Value));
+        if (!*valueAllocCache) throw std::bad_alloc();
+    }
+
+    /* GC_NEXT is a convenience macro for accessing the first word of an object.
+       Take the first list item, advance the list to the next item, and clear the next pointer. */
+    void * p = *valueAllocCache;
+    *valueAllocCache = GC_NEXT(p);
+    GC_NEXT(p) = nullptr;
+#else
+    void * p = allocBytes(sizeof(Value));
+#endif
+
+    nrValues++;
+    return (Value *) p;
+}
+
+
+[[gnu::always_inline]]
+Env & EvalState::allocEnv(size_t size)
+{
+    nrEnvs++;
+    nrValuesInEnvs += size;
+
+    Env * env;
+
+#if HAVE_BOEHMGC
+    if (size == 1) {
+        /* see allocValue for explanations. */
+        if (!*env1AllocCache) {
+            *env1AllocCache = GC_malloc_many(sizeof(Env) + sizeof(Value *));
+            if (!*env1AllocCache) throw std::bad_alloc();
+        }
+
+        void * p = *env1AllocCache;
+        *env1AllocCache = GC_NEXT(p);
+        GC_NEXT(p) = nullptr;
+        env = (Env *) p;
+    } else
+#endif
+        env = (Env *) allocBytes(sizeof(Env) + size * sizeof(Value *));
+
+    env->type = Env::Plain;
+
+    /* We assume that env->values has been cleared by the allocator; maybeThunk() and lookupVar fromWith expect this. */
+
+    return *env;
+}
+
+
+[[gnu::always_inline]]
 void EvalState::forceValue(Value & v, const Pos & pos)
 {
     forceValue(v, [&]() { return pos; });
@@ -52,6 +127,7 @@ void EvalState::forceValue(Value & v, Callable getPos)
 }
 
 
+[[gnu::always_inline]]
 inline void EvalState::forceAttrs(Value & v, const Pos & pos)
 {
     forceAttrs(v, [&]() { return pos; });
@@ -59,6 +135,7 @@ inline void EvalState::forceAttrs(Value & v, const Pos & pos)
 
 
 template <typename Callable>
+[[gnu::always_inline]]
 inline void EvalState::forceAttrs(Value & v, Callable getPos)
 {
     forceValue(v, getPos);
@@ -67,6 +144,7 @@ inline void EvalState::forceAttrs(Value & v, Callable getPos)
 }
 
 
+[[gnu::always_inline]]
 inline void EvalState::forceList(Value & v, const Pos & pos)
 {
     forceValue(v, pos);
@@ -74,18 +152,5 @@ inline void EvalState::forceList(Value & v, const Pos & pos)
         throwTypeError(pos, "value is %1% while a list was expected", v);
 }
 
-/* Note: Various places expect the allocated memory to be zeroed. */
-inline void * allocBytes(size_t n)
-{
-    void * p;
-#if HAVE_BOEHMGC
-    p = GC_MALLOC(n);
-#else
-    p = calloc(n, 1);
-#endif
-    if (!p) throw std::bad_alloc();
-    return p;
-}
-
 
 }

src/libexpr/eval.cc

@@ -449,8 +449,10 @@ EvalState::EvalState(
     , regexCache(makeRegexCache())
 #if HAVE_BOEHMGC
     , valueAllocCache(std::allocate_shared<void *>(traceable_allocator<void *>(), nullptr))
+    , env1AllocCache(std::allocate_shared<void *>(traceable_allocator<void *>(), nullptr))
 #else
     , valueAllocCache(std::make_shared<void *>(nullptr))
+    , env1AllocCache(std::make_shared<void *>(nullptr))
 #endif
     , baseEnv(allocEnv(128))
     , staticBaseEnv(false, 0)
@@ -876,42 +878,6 @@ inline Value * EvalState::lookupVar(Env * env, const ExprVar & var, bool noEval)
 }
 
 
-Value * EvalState::allocValue()
-{
-    /* We use the boehm batch allocator to speed up allocations of Values (of which there are many).
-       GC_malloc_many returns a linked list of objects of the given size, where the first word
-       of each object is also the pointer to the next object in the list. This also means that we
-       have to explicitly clear the first word of every object we take. */
-    if (!*valueAllocCache) {
-        *valueAllocCache = GC_malloc_many(sizeof(Value));
-        if (!*valueAllocCache) throw std::bad_alloc();
-    }
-
-    /* GC_NEXT is a convenience macro for accessing the first word of an object.
-       Take the first list item, advance the list to the next item, and clear the next pointer. */
-    void * p = *valueAllocCache;
-    GC_PTR_STORE_AND_DIRTY(&*valueAllocCache, GC_NEXT(p));
-    GC_NEXT(p) = nullptr;
-
-    nrValues++;
-    auto v = (Value *) p;
-    return v;
-}
-
-
-Env & EvalState::allocEnv(size_t size)
-{
-    nrEnvs++;
-    nrValuesInEnvs += size;
-    Env * env = (Env *) allocBytes(sizeof(Env) + size * sizeof(Value *));
-    env->type = Env::Plain;
-
-    /* We assume that env->values has been cleared by the allocator; maybeThunk() and lookupVar fromWith expect this. */
-
-    return *env;
-}
-
-
 void EvalState::mkList(Value & v, size_t size)
 {
     v.mkList(size);

src/libexpr/eval.hh

@@ -133,9 +133,14 @@ private:
     /* Cache used by prim_match(). */
     std::shared_ptr<RegexCache> regexCache;
 
+#if HAVE_BOEHMGC
     /* Allocation cache for GC'd Value objects. */
     std::shared_ptr<void *> valueAllocCache;
 
+    /* Allocation cache for size-1 Env objects. */
+    std::shared_ptr<void *> env1AllocCache;
+#endif
+
 public:
 
     EvalState(
@@ -347,8 +352,8 @@ public:
     void autoCallFunction(Bindings & args, Value & fun, Value & res);
 
     /* Allocation primitives. */
-    Value * allocValue();
+    inline Value * allocValue();
-    Env & allocEnv(size_t size);
+    inline Env & allocEnv(size_t size);
 
     Value * allocAttr(Value & vAttrs, const Symbol & name);
     Value * allocAttr(Value & vAttrs, std::string_view name);
@@ -509,3 +514,5 @@ extern EvalSettings evalSettings;
 static const std::string corepkgsPrefix{"/__corepkgs__/"};
 
 }
+
+#include "eval-inline.hh"

src/libexpr/nixexpr.hh

@@ -23,14 +23,13 @@ MakeError(RestrictedPathError, Error);
 
 struct Pos
 {
-    FileOrigin origin;
     Symbol file;
-    unsigned int line, column;
+    uint32_t line;
-
+    FileOrigin origin:2;
-    Pos() : origin(foString), line(0), column(0) { }
+    uint32_t column:30;
-    Pos(FileOrigin origin, const Symbol & file, unsigned int line, unsigned int column)
+    Pos() : line(0), origin(foString), column(0) { };
-        : origin(origin), file(file), line(line), column(column) { }
+    Pos(FileOrigin origin, const Symbol & file, uint32_t line, uint32_t column)
-
+        : file(file), line(line), origin(origin), column(column) { };
     operator bool() const
     {
         return line != 0;

src/libutil/error.hh

@@ -54,6 +54,7 @@ typedef enum {
     lvlVomit
 } Verbosity;
 
+/* adjust Pos::origin bit width when adding stuff here */
 typedef enum {
     foFile,
     foStdin,

src/libutil/finally.hh

@@ -1,14 +1,13 @@
 #pragma once
 
-#include <functional>
-
 /* A trivial class to run a function at the end of a scope. */
+template<typename Fn>
 class Finally
 {
 private:
-    std::function<void()> fun;
+    Fn fun;
 
 public:
-    Finally(std::function<void()> fun) : fun(fun) { }
+    Finally(Fn fun) : fun(std::move(fun)) { }
     ~Finally() { fun(); }
 };

src/libutil/util.cc

@@ -682,7 +682,14 @@ std::string drainFD(int fd, bool block, const size_t reserveSize)
 
 void drainFD(int fd, Sink & sink, bool block)
 {
-    int saved;
+    // silence GCC maybe-uninitialized warning in finally
+    int saved = 0;
+
+    if (!block) {
+        saved = fcntl(fd, F_GETFL);
+        if (fcntl(fd, F_SETFL, saved | O_NONBLOCK) == -1)
+            throw SysError("making file descriptor non-blocking");
+    }
 
     Finally finally([&]() {
         if (!block) {
@@ -691,12 +698,6 @@ void drainFD(int fd, Sink & sink, bool block)
         }
     });
 
-    if (!block) {
-        saved = fcntl(fd, F_GETFL);
-        if (fcntl(fd, F_SETFL, saved | O_NONBLOCK) == -1)
-            throw SysError("making file descriptor non-blocking");
-    }
-
     std::vector<unsigned char> buf(64 * 1024);
     while (1) {
         checkInterrupt();