From dfedbc154f08bc025706847b275333526f87579b Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sat, 11 May 2024 00:49:22 +0200 Subject: [PATCH] remove sourceToSink, sinkToSource, and boehm patch Change-Id: I1379841299713175d0225b82a67f50660f9eb5e2 --- boehmgc-coroutine-sp-fallback.diff | 54 --------- package.nix | 11 +- src/libexpr/eval.cc | 64 ----------- src/libutil/serialise.cc | 171 ----------------------------- src/libutil/serialise.hh | 8 -- 5 files changed, 1 insertion(+), 307 deletions(-) delete mode 100644 boehmgc-coroutine-sp-fallback.diff diff --git a/boehmgc-coroutine-sp-fallback.diff b/boehmgc-coroutine-sp-fallback.diff deleted file mode 100644 index a53b7f1f5..000000000 --- a/boehmgc-coroutine-sp-fallback.diff +++ /dev/null @@ -1,54 +0,0 @@ -diff --git a/pthread_stop_world.c b/pthread_stop_world.c -index 2b45489..0e6d8ef 100644 ---- a/pthread_stop_world.c -+++ b/pthread_stop_world.c -@@ -776,6 +776,8 @@ STATIC void GC_restart_handler(int sig) - /* world is stopped. Should not fail if it isn't. */ - GC_INNER void GC_push_all_stacks(void) - { -+ size_t stack_limit; -+ pthread_attr_t pattr; - GC_bool found_me = FALSE; - size_t nthreads = 0; - int i; -@@ -868,6 +870,40 @@ GC_INNER void GC_push_all_stacks(void) - hi = p->altstack + p->altstack_size; - # endif - /* FIXME: Need to scan the normal stack too, but how ? */ -+ } else { -+ #ifdef HAVE_PTHREAD_ATTR_GET_NP -+ if (pthread_attr_init(&pattr) != 0) { -+ ABORT("GC_push_all_stacks: pthread_attr_init failed!"); -+ } -+ if (pthread_attr_get_np(p->id, &pattr) != 0) { -+ ABORT("GC_push_all_stacks: pthread_attr_get_np failed!"); -+ } -+ #else -+ if (pthread_getattr_np(p->id, &pattr)) { -+ ABORT("GC_push_all_stacks: pthread_getattr_np failed!"); -+ } -+ #endif -+ if (pthread_attr_getstacksize(&pattr, &stack_limit)) { -+ ABORT("GC_push_all_stacks: pthread_attr_getstacksize failed!"); -+ } -+ if (pthread_attr_destroy(&pattr)) { -+ ABORT("GC_push_all_stacks: pthread_attr_destroy failed!"); -+ } -+ // When a thread goes into a coroutine, we lose its original sp until -+ // control flow returns to the thread. -+ // While in the coroutine, the sp points outside the thread stack, -+ // so we can detect this and push the entire thread stack instead, -+ // as an approximation. -+ // We assume that the coroutine has similarly added its entire stack. -+ // This could be made accurate by cooperating with the application -+ // via new functions and/or callbacks. -+ #ifndef STACK_GROWS_UP -+ if (lo >= hi || lo < hi - stack_limit) { // sp outside stack -+ lo = hi - stack_limit; -+ } -+ #else -+ #error "STACK_GROWS_UP not supported in boost_coroutine2 (as of june 2021), so we don't support it in Nix." -+ #endif - } - # ifdef STACKPTR_CORRECTOR_AVAILABLE - if (GC_sp_corrector != 0) diff --git a/package.nix b/package.nix index 453a5f807..435a265d1 100644 --- a/package.nix +++ b/package.nix @@ -62,15 +62,7 @@ __forDefaults ? { canRunInstalled = stdenv.buildPlatform.canExecute stdenv.hostPlatform; - boehmgc-nix = (boehmgc.override { enableLargeConfig = true; }).overrideAttrs { - patches = [ - # We do *not* include prev.patches (which doesn't exist in normal pkgs.boehmgc anyway) - # because if the caller of this package passed a patched boehm as `boehmgc` instead of - # `boehmgc-nix` then this will almost certainly have duplicate patches, which means - # the patches won't apply and we'll get a build failure. - ./boehmgc-coroutine-sp-fallback.diff - ]; - }; + boehmgc-nix = boehmgc.override { enableLargeConfig = true; }; editline-lix = editline.overrideAttrs (prev: { configureFlags = prev.configureFlags or [ ] ++ [ (lib.enableFeature true "sigstop") ]; @@ -167,7 +159,6 @@ stdenv.mkDerivation (finalAttrs: { functionalTestFiles ] ++ lib.optionals (!finalAttrs.dontBuild || internalApiDocs) [ - ./boehmgc-coroutine-sp-fallback.diff ./doc ./misc ./src diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index ced92136b..77b46c2ed 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -192,42 +192,6 @@ static void * oomHandler(size_t requested) throw std::bad_alloc(); } -class BoehmGCStackAllocator : public StackAllocator { - boost::coroutines2::protected_fixedsize_stack stack { - // We allocate 8 MB, the default max stack size on NixOS. - // A smaller stack might be quicker to allocate but reduces the stack - // depth available for source filter expressions etc. - std::max(boost::context::stack_traits::default_size(), static_cast(8 * 1024 * 1024)) - }; - - // This is specific to boost::coroutines2::protected_fixedsize_stack. - // The stack protection page is included in sctx.size, so we have to - // subtract one page size from the stack size. - std::size_t pfss_usable_stack_size(boost::context::stack_context &sctx) { - return sctx.size - boost::context::stack_traits::page_size(); - } - - public: - boost::context::stack_context allocate() override { - auto sctx = stack.allocate(); - - // Stacks generally start at a high address and grow to lower addresses. - // Architectures that do the opposite are rare; in fact so rare that - // boost_routine does not implement it. - // So we subtract the stack size. - GC_add_roots(static_cast(sctx.sp) - pfss_usable_stack_size(sctx), sctx.sp); - return sctx; - } - - void deallocate(boost::context::stack_context sctx) override { - GC_remove_roots(static_cast(sctx.sp) - pfss_usable_stack_size(sctx), sctx.sp); - stack.deallocate(sctx); - } - -}; - -static BoehmGCStackAllocator boehmGCStackAllocator; - #endif @@ -243,23 +207,6 @@ static Symbol getName(const AttrName & name, EvalState & state, Env & env) } } -#if HAVE_BOEHMGC -/* Disable GC while this object lives. Used by CoroutineContext. - * - * Boehm keeps a count of GC_disable() and GC_enable() calls, - * and only enables GC when the count matches. - */ -class BoehmDisableGC { -public: - BoehmDisableGC() { - GC_disable(); - }; - ~BoehmDisableGC() { - GC_enable(); - }; -}; -#endif - static bool gcInitialised = false; void initGC() @@ -281,17 +228,6 @@ void initGC() GC_set_oom_fn(oomHandler); - StackAllocator::defaultAllocator = &boehmGCStackAllocator; - - -#if NIX_BOEHM_PATCH_VERSION != 1 - printTalkative("Unpatched BoehmGC, disabling GC inside coroutines"); - /* Used to disable GC when entering coroutines on macOS */ - create_coro_gc_hook = []() -> std::shared_ptr { - return std::make_shared(); - }; -#endif - /* Set the initial heap size to something fairly big (25% of physical RAM, up to a maximum of 384 MiB) so that in most cases we don't need to garbage collect at all. (Collection has a diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc index 11bc183cc..a294a7ea1 100644 --- a/src/libutil/serialise.cc +++ b/src/libutil/serialise.cc @@ -149,177 +149,6 @@ size_t StringSource::read(char * data, size_t len) } -#if BOOST_VERSION >= 106300 && BOOST_VERSION < 106600 -#error Coroutines are broken in this version of Boost! -#endif - -/* A concrete datatype allow virtual dispatch of stack allocation methods. */ -struct VirtualStackAllocator { - StackAllocator *allocator = StackAllocator::defaultAllocator; - - boost::context::stack_context allocate() { - return allocator->allocate(); - } - - void deallocate(boost::context::stack_context sctx) { - allocator->deallocate(sctx); - } -}; - - -/* This class reifies the default boost coroutine stack allocation strategy with - a virtual interface. */ -class DefaultStackAllocator : public StackAllocator { - boost::coroutines2::default_stack stack; - - boost::context::stack_context allocate() { - return stack.allocate(); - } - - void deallocate(boost::context::stack_context sctx) { - stack.deallocate(sctx); - } -}; - -static DefaultStackAllocator defaultAllocatorSingleton; - -StackAllocator *StackAllocator::defaultAllocator = &defaultAllocatorSingleton; - - -std::shared_ptr (*create_coro_gc_hook)() = []() -> std::shared_ptr { - return {}; -}; - -/* This class is used for entry and exit hooks on coroutines */ -class CoroutineContext { - /* Disable GC when entering the coroutine without the boehm patch, - * since it doesn't find the main thread stack in this case. - * std::shared_ptr performs type-erasure, so it will call the right - * deleter. */ - const std::shared_ptr coro_gc_hook = create_coro_gc_hook(); -public: - CoroutineContext() {}; - ~CoroutineContext() {}; -}; - -std::unique_ptr sourceToSink(std::function fun) -{ - struct SourceToSink : FinishSink - { - typedef boost::coroutines2::coroutine coro_t; - - std::function fun; - std::optional coro; - - SourceToSink(std::function fun) : fun(fun) - { - } - - std::string_view cur; - - void operator () (std::string_view in) override - { - if (in.empty()) return; - cur = in; - - if (!coro) { - CoroutineContext ctx; - coro = coro_t::push_type(VirtualStackAllocator{}, [&](coro_t::pull_type & yield) { - LambdaSource source([&](char *out, size_t out_len) { - if (cur.empty()) { - yield(); - if (yield.get()) { - throw EndOfFile("coroutine exhausted"); - } - } - - size_t n = std::min(cur.size(), out_len); - memcpy(out, cur.data(), n); - cur.remove_prefix(n); - return n; - }); - fun(source); - }); - } - - if (!*coro) { abort(); } - - if (!cur.empty()) { - CoroutineContext ctx; - (*coro)(false); - } - } - - void finish() override - { - if (!coro) return; - if (!*coro) abort(); - { - CoroutineContext ctx; - (*coro)(true); - } - if (*coro) abort(); - } - }; - - return std::make_unique(fun); -} - - -std::unique_ptr sinkToSource(std::function fun) -{ - struct SinkToSource : Source - { - typedef boost::coroutines2::coroutine coro_t; - - std::function fun; - std::optional coro; - - SinkToSource(std::function fun) - : fun(fun) - { - } - - std::string cur; - size_t pos = 0; - - size_t read(char * data, size_t len) override - { - if (!coro) { - CoroutineContext ctx; - coro = coro_t::pull_type(VirtualStackAllocator{}, [&](coro_t::push_type & yield) { - LambdaSink sink([&](std::string_view data) { - if (!data.empty()) yield(std::string(data)); - }); - fun(sink); - }); - } - - if (!*coro) { - throw EndOfFile("coroutine has finished"); - } - - if (pos == cur.size()) { - if (!cur.empty()) { - CoroutineContext ctx; - (*coro)(); - } - cur = coro->get(); - pos = 0; - } - - auto n = std::min(cur.size() - pos, len); - memcpy(data, cur.data() + pos, n); - pos += n; - - return n; - } - }; - - return std::make_unique(fun); -} - - void writePadding(size_t len, Sink & sink) { if (len % 8) { diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh index 874c67b75..9b1892bbb 100644 --- a/src/libutil/serialise.hh +++ b/src/libutil/serialise.hh @@ -362,14 +362,6 @@ private: Bytes buf{}; }; -std::unique_ptr sourceToSink(std::function fun); - -/** - * Convert a function that feeds data into a Sink into a Source. The - * Source executes the function as a coroutine. - */ -std::unique_ptr sinkToSource(std::function fun); - inline Sink & operator<<(Sink & sink, Generator && g) { while (auto buffer = g.next()) {