Feat/execution ctx pool

CHANGELOG.md

@@ -145,6 +145,9 @@
 - renamed almost all builtins to prefix them with `builtin__`, to have them proxied in the standard library (to be able to import and scope them properly)
 - new super instruction `CALL_BUILTIN_WITHOUT_RETURN_ADDRESS` to optimize the proxied builtins, skipping the return address deletion
 - the VM no longer store a reference to the current function being called in the newly created scope
+- execution contexts can be reused for async calls if they are not active, to avoid constantly requesting memory and creating (heavy) contexts
+  - if there is more than 5 contexts, the 6th one will be destroyed once it completes
+- execution contexts are now marked as free to be reused (or deleted) once a value has been computed, without waiting for a call to `await`
 
 ### Removed
 - removed unused `NodeType::Closure`

include/Ark/Module.hpp

@@ -24,7 +24,7 @@ namespace Ark
 // the same keyword is used for both importing and exporting
 #        define ARK_API extern "C" __attribute__((__visibility__("default")))
 #    else
-// GCC < 4 has no mechanism to explicitely hide symbols, everything's exported
+// GCC < 4 has no mechanism to explicitly hide symbols, everything's exported
 #        define ARK_API extern "C"
 #    endif
 #endif

include/Ark/Utils/Logger.hpp

@@ -74,7 +74,7 @@ namespace Ark::internal
          * @param args
          */
         template <typename... Args>
-        void debug(const Logger::MessageAndLocation data, Args&&... args)
+        void debug(const Logger::MessageAndLocation& data, Args&&... args)
         {
             if (shouldDebug())
                 fmt::println(

include/Ark/VM/ExecutionContext.hpp

@@ -15,6 +15,7 @@
 #include <limits>
 #include <memory>
 #include <optional>
+#include <atomic>
 
 #include <Ark/Constants.hpp>
 #include <Ark/VM/Value.hpp>
@@ -37,6 +38,7 @@ namespace Ark::internal
         uint16_t fc {};     ///< Frame count
         uint16_t last_symbol;
         const bool primary;  ///< Tells if the current ExecutionContext is the primary one or not
+        std::atomic_bool active;
 
         std::optional<ClosureScope> saved_scope {};                            ///< Scope created by CAPTURE <x> instructions, used by the MAKE_CLOSURE instruction
         std::vector<std::shared_ptr<ClosureScope>> stacked_closure_scopes {};  ///< Stack the closure scopes to keep the closure alive as long as we are calling them
@@ -50,8 +52,19 @@ namespace Ark::internal
             last_symbol(std::numeric_limits<uint16_t>::max()),
             primary(Count == 0)
         {
+            active.store(true);
             Count++;
         }
+
+        [[nodiscard]] bool isFree() const
+        {
+            return !active.load();
+        }
+
+        void setActive(const bool toggle)
+        {
+            active.store(toggle);
+        }
     };
 }
 

include/Ark/VM/Future.hpp

@@ -37,8 +37,6 @@ namespace Ark::internal
         Value resolve();
 
     private:
-        ExecutionContext* m_context;
-        VM* m_vm;
         std::future<Value> m_value;  ///< The actual thread
     };
 }

include/Ark/VM/ScopeView.hpp

@@ -64,23 +64,23 @@ namespace Ark::internal
          * @return true On success
          * @return false Otherwise
          */
-        bool maybeHas(uint16_t id) const noexcept;
+        [[nodiscard]] bool maybeHas(uint16_t id) const noexcept;
 
         /**
          * @brief Get a value from its symbol id
          *
          * @param id_to_look_for
          * @return Value* Returns nullptr if the value can not be found
          */
-        Value* operator[](uint16_t id_to_look_for) noexcept;
+        [[nodiscard]] Value* operator[](uint16_t id_to_look_for) noexcept;
 
         /**
          * @brief Get a value from its symbol id
          *
          * @param id_to_look_for
          * @return const Value* Returns nullptr if the value can not be found
          */
-        const Value* operator[](uint16_t id_to_look_for) const noexcept;
+        [[nodiscard]] const Value* operator[](uint16_t id_to_look_for) const noexcept;
 
         /**
          * @brief Get the id of a variable based on its value ; used for debug only

src/arkreactor/VM/Future.cpp

@@ -4,11 +4,21 @@
 
 namespace Ark::internal
 {
-    Future::Future(ExecutionContext* context, VM* vm, std::vector<Value>& args) :
-        m_context(context), m_vm(vm), m_value(std::async(std::launch::async, [vm, context, args]() mutable {
-            return vm->resolve(context, args);
-        }))
-    {}
+    Future::Future(ExecutionContext* context, VM* vm, std::vector<Value>& args)
+    {
+        m_value = std::async(
+            std::launch::async,
+            [vm, context, args]() mutable {
+                const Value res = vm->resolve(context, args);
+                // We need to mark the context as free to use as soon as possible,
+                // because if we do it in `Future::resolve`, it will never be marked as free
+                // if the future is not awaited, even though it can already be reused.
+                // The value is returned as a copy by VM::resolve, and not a reference,
+                // thus it is okay to get rid of the context.
+                vm->deleteContext(context);
+                return res;
+            });
+    }
 
     Value Future::resolve()
     {
@@ -17,10 +27,6 @@ namespace Ark::internal
 
         m_value.wait();
         Value res = m_value.get();
-
-        m_vm->deleteContext(m_context);
-        m_context = nullptr;
-
         return res;
     }
 }

src/arkreactor/VM/VM.cpp

@@ -344,18 +344,51 @@ namespace Ark
     {
         const std::lock_guard lock(m_mutex);
 
-        m_execution_contexts.push_back(std::make_unique<ExecutionContext>());
-        ExecutionContext* ctx = m_execution_contexts.back().get();
+        ExecutionContext* ctx = nullptr;
+
+        // Try and find a free execution context.
+        // If there is only one context, this is the primary one, which can't be reused.
+        // Otherwise, we can check if a context is marked as free and reserve it!
+        // It is possible that all contexts are being used, thus we will create one (active by default) in that case.
+
+        if (m_execution_contexts.size() > 1)
+        {
+            for (std::size_t i = 0; i < m_execution_contexts.size(); ++i)
+            {
+                if (!m_execution_contexts[i]->primary && m_execution_contexts[i]->isFree())
+                {
+                    ctx = m_execution_contexts[i].get();
+                    ctx->setActive(true);
+                    // reset the context before using it
+                    ctx->sp = 0;
+                    ctx->saved_scope.reset();
+                    ctx->stacked_closure_scopes.clear();
+                    ctx->locals.clear();
+                    break;
+                }
+            }
+        }
+
+        if (ctx == nullptr)
+            ctx = m_execution_contexts.emplace_back(std::make_unique<ExecutionContext>()).get();
+
+        assert(!ctx->primary && "The new context shouldn't be marked as primary!");
+        assert(ctx != m_execution_contexts.front().get() && "The new context isn't really new!");
+
+        const ExecutionContext& primary_ctx = *m_execution_contexts.front();
+        ctx->locals.reserve(primary_ctx.locals.size());
+        ctx->scopes_storage = primary_ctx.scopes_storage;
         ctx->stacked_closure_scopes.emplace_back(nullptr);
+        ctx->fc = 1;
 
-        ctx->locals.reserve(m_execution_contexts.front()->locals.size());
-        ctx->scopes_storage = m_execution_contexts.front()->scopes_storage;
-        for (const auto& local : m_execution_contexts.front()->locals)
+        for (const auto& scope_view : primary_ctx.locals)
         {
-            auto& scope = ctx->locals.emplace_back(ctx->scopes_storage.data(), local.m_start);
-            scope.m_size = local.m_size;
-            scope.m_min_id = local.m_min_id;
-            scope.m_max_id = local.m_max_id;
+            auto& new_scope = ctx->locals.emplace_back(ctx->scopes_storage.data(), scope_view.m_start);
+            for (std::size_t i = 0; i < scope_view.size(); ++i)
+            {
+                const auto& [id, val] = scope_view.atPos(i);
+                new_scope.push_back(id, val);
+            }
         }
 
         return ctx;
@@ -365,14 +398,30 @@ namespace Ark
     {
         const std::lock_guard lock(m_mutex);
 
-        const auto it =
-            std::ranges::remove_if(
-                m_execution_contexts,
-                [ec](const std::unique_ptr<ExecutionContext>& ctx) {
-                    return ctx.get() == ec;
-                })
-                .begin();
-        m_execution_contexts.erase(it);
+        // 1 + 4 additional contexts, it's a bit much (~600kB per context) to have in memory
+        if (m_execution_contexts.size() > 5)
+        {
+            const auto it =
+                std::ranges::remove_if(
+                    m_execution_contexts,
+                    [ec](const std::unique_ptr<ExecutionContext>& ctx) {
+                        return ctx.get() == ec;
+                    })
+                    .begin();
+            m_execution_contexts.erase(it);
+        }
+        else
+        {
+            // mark the used context as ready to be used again
+            for (std::size_t i = 1; i < m_execution_contexts.size(); ++i)
+            {
+                if (m_execution_contexts[i].get() == ec)
+                {
+                    ec->setActive(false);
+                    break;
+                }
+            }
+        }
     }
 
     Future* VM::createFuture(std::vector<Value>& args)

src/arkreactor/VM/Value/Dict.cpp

@@ -32,9 +32,8 @@ namespace Ark::internal
     {
         std::vector<Value> keys;
         keys.reserve(m_dict.size());
+        std::ranges::copy(std::ranges::views::keys(m_dict), std::back_inserter(keys));
 
-        for (auto&& key : std::ranges::views::keys(m_dict))
-            keys.push_back(key);
         return keys;
     }
 

tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_add_num.ark

@@ -0,0 +1 @@
+(builtin__dict:add 1)

tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_add_num.expected

@@ -0,0 +1,9 @@
+Function dict:add expected 3 arguments but got 1
+  -> dictionary (Dict) was of type Number
+  -> key (any) was not provided
+  -> value (any) was not provided
+
+In file tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_add_num.ark:1
+    1 | (builtin__dict:add 1)
+      | ^~~~~~~~~~~~~~~~~~~~
+    2 |

tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_contains_num.ark

@@ -0,0 +1 @@
+(builtin__dict:contains 1)

tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_contains_num.expected

@@ -0,0 +1,8 @@
+Function dict:contains expected 2 arguments but got 1
+  -> dictionary (Dict) was of type Number
+  -> key (any) was not provided
+
+In file tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_contains_num.ark:1
+    1 | (builtin__dict:contains 1)
+      | ^~~~~~~~~~~~~~~~~~~~~~~~~
+    2 |

tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_get_num.ark

@@ -0,0 +1 @@
+(builtin__dict:get 1)

tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_get_num.expected

@@ -0,0 +1,8 @@
+Function dict:get expected 2 arguments but got 1
+  -> dictionary (Dict) was of type Number
+  -> key (any) was not provided
+
+In file tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_get_num.ark:1
+    1 | (builtin__dict:get 1)
+      | ^~~~~~~~~~~~~~~~~~~~
+    2 |

tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_keys_num.ark

@@ -0,0 +1 @@
+(builtin__dict:keys 1)

tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_keys_num.expected

@@ -0,0 +1,7 @@
+Function dict:keys expected 1 argument
+  -> dictionary (Dict) was of type Number
+
+In file tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_keys_num.ark:1
+    1 | (builtin__dict:keys 1)
+      | ^~~~~~~~~~~~~~~~~~~~~
+    2 |

tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_odd_args.ark

@@ -0,0 +1 @@
+(dict 1)

tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_odd_args.expected

@@ -0,0 +1,6 @@
+dict: require an even number of argument to construct (key, value) pairs and create a dictionary. Got 1 argument
+
+In file tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_odd_args.ark:1
+    1 | (dict 1)
+      | ^~~~~~~
+    2 |

tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_remove_num.ark

@@ -0,0 +1 @@
+(builtin__dict:remove 1)

tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_remove_num.expected

@@ -0,0 +1,8 @@
+Function dict:remove expected 2 arguments but got 1
+  -> dictionary (Dict) was of type Number
+  -> key (any) was not provided
+
+In file tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_remove_num.ark:1
+    1 | (builtin__dict:remove 1)
+      | ^~~~~~~~~~~~~~~~~~~~~~~
+    2 |

tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_size_num.ark

@@ -0,0 +1 @@
+(builtin__dict:size 1)

tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_size_num.expected

@@ -0,0 +1,7 @@
+Function dict:size expected 1 argument
+  -> dictionary (Dict) was of type Number
+
+In file tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_size_num.ark:1
+    1 | (builtin__dict:size 1)
+      | ^~~~~~~~~~~~~~~~~~~~~
+    2 |

tests/unittests/resources/LangSuite/async-tests.ark

@@ -1,5 +1,6 @@
 (import std.Testing)
 (import std.List)
+(import std.Sys :sleep)
 
 (let foo (fun (a b) (+ a b)))
 (let async-foo (async foo 1 2))
@@ -37,4 +38,20 @@
         (let time-async (- (time) start-async))
 
         (test:eq 1000 res-async)
-        (test:eq 1000 res-non-async)})})
+        (test:eq 1000 res-non-async)})
+
+    (test:case "execution contexts get reused" {
+        # this forces the VM to create more than 5 execution context,
+        # and delete the 6th
+        (async sleep 128)
+        (async sleep 128)
+        (async sleep 128)
+        (async sleep 128)
+        (async sleep 128)
+        (sleep 250)  # wait for the async call to complete
+
+        # we have 5 contexts here, and 4 that are free to be (re)used
+        (async sleep 128) (sleep 200)
+        (async sleep 128) (sleep 200)
+        (async sleep 128) (sleep 200)
+        (async sleep 128) (sleep 200) }) })