diff --git a/CHANGELOG.md b/CHANGELOG.md index fbc3a11e..db4f1420 100644 --- a/CHANGELOG.md +++ b/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` diff --git a/include/Ark/Module.hpp b/include/Ark/Module.hpp index fd91adcf..e0b4c9a9 100644 --- a/include/Ark/Module.hpp +++ b/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 diff --git a/include/Ark/Utils/Logger.hpp b/include/Ark/Utils/Logger.hpp index 02fe6a0a..1e614138 100644 --- a/include/Ark/Utils/Logger.hpp +++ b/include/Ark/Utils/Logger.hpp @@ -74,7 +74,7 @@ namespace Ark::internal * @param args */ template - void debug(const Logger::MessageAndLocation data, Args&&... args) + void debug(const Logger::MessageAndLocation& data, Args&&... args) { if (shouldDebug()) fmt::println( diff --git a/include/Ark/VM/ExecutionContext.hpp b/include/Ark/VM/ExecutionContext.hpp index 53825eac..bd864d52 100644 --- a/include/Ark/VM/ExecutionContext.hpp +++ b/include/Ark/VM/ExecutionContext.hpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -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 saved_scope {}; ///< Scope created by CAPTURE instructions, used by the MAKE_CLOSURE instruction std::vector> 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::max()), primary(Count == 0) { + active.store(true); Count++; } + + [[nodiscard]] bool isFree() const + { + return !active.load(); + } + + void setActive(const bool toggle) + { + active.store(toggle); + } }; } diff --git a/include/Ark/VM/Future.hpp b/include/Ark/VM/Future.hpp index 1c1d47b6..60110210 100644 --- a/include/Ark/VM/Future.hpp +++ b/include/Ark/VM/Future.hpp @@ -37,8 +37,6 @@ namespace Ark::internal Value resolve(); private: - ExecutionContext* m_context; - VM* m_vm; std::future m_value; ///< The actual thread }; } diff --git a/include/Ark/VM/ScopeView.hpp b/include/Ark/VM/ScopeView.hpp index ebef505d..43f85d3a 100644 --- a/include/Ark/VM/ScopeView.hpp +++ b/include/Ark/VM/ScopeView.hpp @@ -64,7 +64,7 @@ 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 @@ -72,7 +72,7 @@ namespace Ark::internal * @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 @@ -80,7 +80,7 @@ namespace Ark::internal * @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 diff --git a/lib/std b/lib/std index c13f009a..0b9d2e7f 160000 --- a/lib/std +++ b/lib/std @@ -1 +1 @@ -Subproject commit c13f009a485224876b8e20b8d9dcf9c606b3f407 +Subproject commit 0b9d2e7f7cbd1e606163c0ee3c3f212ad2df8049 diff --git a/src/arkreactor/Builtins/Mathematics.cpp b/src/arkreactor/Builtins/Math.cpp similarity index 100% rename from src/arkreactor/Builtins/Mathematics.cpp rename to src/arkreactor/Builtins/Math.cpp diff --git a/src/arkreactor/VM/Future.cpp b/src/arkreactor/VM/Future.cpp index 26a989e6..6c11d5fd 100644 --- a/src/arkreactor/VM/Future.cpp +++ b/src/arkreactor/VM/Future.cpp @@ -4,11 +4,21 @@ namespace Ark::internal { - Future::Future(ExecutionContext* context, VM* vm, std::vector& 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& 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; } } diff --git a/src/arkreactor/VM/VM.cpp b/src/arkreactor/VM/VM.cpp index dd8f027a..e77f271b 100644 --- a/src/arkreactor/VM/VM.cpp +++ b/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* 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()).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& 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& 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& args) diff --git a/src/arkreactor/VM/Value/Dict.cpp b/src/arkreactor/VM/Value/Dict.cpp index 1a15a3f6..affd16f5 100644 --- a/src/arkreactor/VM/Value/Dict.cpp +++ b/src/arkreactor/VM/Value/Dict.cpp @@ -32,9 +32,8 @@ namespace Ark::internal { std::vector 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; } diff --git a/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_add_num.ark b/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_add_num.ark new file mode 100644 index 00000000..816adf2d --- /dev/null +++ b/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_add_num.ark @@ -0,0 +1 @@ +(builtin__dict:add 1) diff --git a/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_add_num.expected b/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_add_num.expected new file mode 100644 index 00000000..e125090d --- /dev/null +++ b/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 | diff --git a/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_contains_num.ark b/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_contains_num.ark new file mode 100644 index 00000000..e8ffc5bb --- /dev/null +++ b/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_contains_num.ark @@ -0,0 +1 @@ +(builtin__dict:contains 1) diff --git a/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_contains_num.expected b/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_contains_num.expected new file mode 100644 index 00000000..509c5917 --- /dev/null +++ b/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 | diff --git a/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_get_num.ark b/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_get_num.ark new file mode 100644 index 00000000..d632d0a9 --- /dev/null +++ b/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_get_num.ark @@ -0,0 +1 @@ +(builtin__dict:get 1) diff --git a/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_get_num.expected b/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_get_num.expected new file mode 100644 index 00000000..c7ae95cb --- /dev/null +++ b/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 | diff --git a/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_keys_num.ark b/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_keys_num.ark new file mode 100644 index 00000000..84a36b70 --- /dev/null +++ b/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_keys_num.ark @@ -0,0 +1 @@ +(builtin__dict:keys 1) diff --git a/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_keys_num.expected b/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_keys_num.expected new file mode 100644 index 00000000..ecf7691c --- /dev/null +++ b/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 | diff --git a/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_odd_args.ark b/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_odd_args.ark new file mode 100644 index 00000000..7bb820a0 --- /dev/null +++ b/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_odd_args.ark @@ -0,0 +1 @@ +(dict 1) diff --git a/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_odd_args.expected b/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_odd_args.expected new file mode 100644 index 00000000..7a4689b3 --- /dev/null +++ b/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 | diff --git a/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_remove_num.ark b/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_remove_num.ark new file mode 100644 index 00000000..080a9694 --- /dev/null +++ b/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_remove_num.ark @@ -0,0 +1 @@ +(builtin__dict:remove 1) diff --git a/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_remove_num.expected b/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_remove_num.expected new file mode 100644 index 00000000..1fa27603 --- /dev/null +++ b/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 | diff --git a/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_size_num.ark b/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_size_num.ark new file mode 100644 index 00000000..0f7be6d8 --- /dev/null +++ b/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_size_num.ark @@ -0,0 +1 @@ +(builtin__dict:size 1) diff --git a/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_size_num.expected b/tests/unittests/resources/DiagnosticsSuite/typeChecking/dict_size_num.expected new file mode 100644 index 00000000..131d1c23 --- /dev/null +++ b/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 | diff --git a/tests/unittests/resources/LangSuite/async-tests.ark b/tests/unittests/resources/LangSuite/async-tests.ark index f80e2b9d..9c79247c 100644 --- a/tests/unittests/resources/LangSuite/async-tests.ark +++ b/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) }) })