From 332a8028594745ffecf6b3acaa5705158f1568cf Mon Sep 17 00:00:00 2001 From: Alexandre Plateau Date: Sat, 20 Apr 2024 15:09:18 +0200 Subject: [PATCH 1/7] refactor(vm): remove ARK_PROFILER_COUNT define --- CHANGELOG.md | 1 + CMakeLists.txt | 5 ----- README.md | 1 - include/Ark/VM/Value.hpp | 15 ++----------- src/arkreactor/VM/Value.cpp | 42 +------------------------------------ src/arkscript/main.cpp | 12 +---------- 6 files changed, 5 insertions(+), 71 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5112878e6..b90bf25a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ - `Utils::digPlaces` and `Utils::decPlaces` got removed as they were no longer needed - removed deprecated code (`list:removeAt`, `ark` executable now replaced by `arkscript`) - removed `VM::getUserPointer` and `VM::setUserPointer` +- removed `ARK_PROFILER_COUNT` define ## [3.5.0] - 2023-02-19 ### Added diff --git a/CMakeLists.txt b/CMakeLists.txt index f1dd0c6a9..c4b075b79 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -145,7 +145,6 @@ target_compile_definitions(ArkReactor PRIVATE ARK_EXPORT) option(ARK_BUILD_EXE "Build a standalone arkscript executable" Off) option(ARK_ENABLE_SYSTEM "Enable sys:exec" On) # enable use of (sys:exec "command here") -option(ARK_PROFILER_COUNT "Enable creations/copies/moves counting on the Value" Off) option(ARK_PROFILER_MIPS "Enable MIPS calculation" Off) option(ARK_NO_STDLIB "Do not install the standard library with the Ark library" Off) option(ARK_BUILD_MODULES "Build the std library modules or not" Off) @@ -153,10 +152,6 @@ option(ARK_SANITIZERS "Enable ASAN and UBSAN" Off) option(ARK_TESTS "Build ArkScript unit tests" Off) option(ARK_BENCHMARKS "Build ArkScript benchmarks" Off) -if (ARK_PROFILER_COUNT) - target_compile_definitions(ArkReactor PRIVATE -DARK_PROFILER_COUNT) -endif () - if (ARK_ENABLE_SYSTEM) target_compile_definitions(ArkReactor PRIVATE -DARK_ENABLE_SYSTEM) endif () diff --git a/README.md b/README.md index 4b035555a..db6e116e2 100644 --- a/README.md +++ b/README.md @@ -156,7 +156,6 @@ Different CMake switches are available to customize the build: * `-DARK_BUILD_EXE` to generate an executable, defaults to Off, building a shared library only * `-DARK_ENABLE_SYSTEM` to enable `sys:exec` (execute shell commands without restrictions), defaults to On -* `-DARK_PROFILER_COUNT` to count every creation/copy/move of the internal value type, defaults to Off * `-DARK_PROFILER_MIPS` to enable the MIPS counting, defaults to Off * `-DARK_NO_STDLIB` to avoid the installation of the ArkScript standard library * `-DARK_BUILD_MODULES` to trigger the modules build diff --git a/include/Ark/VM/Value.hpp b/include/Ark/VM/Value.hpp index b6d77f32a..982641c69 100644 --- a/include/Ark/VM/Value.hpp +++ b/include/Ark/VM/Value.hpp @@ -61,15 +61,10 @@ namespace Ark "InstPtr" }; -// for debugging purposes only -#ifdef ARK_PROFILER_COUNT - extern unsigned value_creations, value_copies, value_moves; -#endif - class ARK_API Value { public: - using ProcType = Value (*)(std::vector&, VM*); // std::function&, VM*)> + using ProcType = Value (*)(std::vector&, VM*); using Iterator = std::vector::iterator; using ConstIterator = std::vector::const_iterator; @@ -110,15 +105,9 @@ namespace Ark template Value(ValueType type, T&& value) noexcept : m_const_type(static_cast(type)), - m_value(std::move(value)) + m_value(value) {} -#ifdef ARK_PROFILER_COUNT - Value(const Value& val) noexcept; - Value(Value&& other) noexcept; - Value& operator=(const Value& other) noexcept; -#endif - /** * @brief Construct a new Value object as a Number * diff --git a/src/arkreactor/VM/Value.cpp b/src/arkreactor/VM/Value.cpp index 27fd97136..7826c2355 100644 --- a/src/arkreactor/VM/Value.cpp +++ b/src/arkreactor/VM/Value.cpp @@ -2,7 +2,7 @@ #include -#define init_const_type(is_const, type) ((is_const ? (1 << 7) : 0) | static_cast(type)) +#define init_const_type(is_const, type) (((is_const) ? (1 << 7) : 0) | static_cast(type)) namespace Ark { @@ -10,8 +10,6 @@ namespace Ark m_const_type(init_const_type(false, ValueType::Undefined)) {} - // -------------------------- - Value::Value(ValueType type) noexcept : m_const_type(init_const_type(false, type)) { @@ -19,45 +17,7 @@ namespace Ark m_value = std::vector(); else if (type == ValueType::String) m_value = ""; - -#ifdef ARK_PROFILER_COUNT - value_creations++; -#endif - } - -#ifdef ARK_PROFILER_COUNT - extern unsigned value_creations = 0; - extern unsigned value_copies = 0; - extern unsigned value_moves = 0; - - Value::Value(const Value& val) noexcept : - m_value(val.m_value), - m_const_type(val.m_const_type) - { - if (valueType() != ValueType::Reference) - value_copies++; - } - - Value::Value(Value&& other) noexcept - { - m_value = std::move(other.m_value); - m_const_type = std::move(other.m_const_type); - - if (valueType() != ValueType::Reference) - value_moves++; - } - - Value& Value::operator=(const Value& other) noexcept - { - m_value = other.m_value; - m_const_type = other.m_const_type; - - if (valueType() != ValueType::Reference) - value_copies++; - - return *this; } -#endif Value::Value(int value) noexcept : m_const_type(init_const_type(false, ValueType::Number)), m_value(static_cast(value)) diff --git a/src/arkscript/main.cpp b/src/arkscript/main.cpp index 474c38fb9..3f1b0f2a0 100644 --- a/src/arkscript/main.cpp +++ b/src/arkscript/main.cpp @@ -229,17 +229,7 @@ int main(int argc, char** argv) return -1; Ark::VM vm(state); - int out = vm.run(); - -#ifdef ARK_PROFILER_COUNT - std::cout << "\n\nValue\n=====\n" - << "\tCreations: " << Ark::internal::value_creations - << "\n\tCopies: " << Ark::internal::value_copies - << "\n\tMoves: " << Ark::internal::value_moves - << "\n\n\tCopy coeff: " << static_cast(Ark::internal::value_copies) / Ark::internal::value_creations; -#endif - - return out; + return vm.run(); } case mode::eval: From bf743c34be0f96f7a455c2f035258d9f8e0a43ec Mon Sep 17 00:00:00 2001 From: Alexandre Plateau Date: Sat, 20 Apr 2024 15:34:41 +0200 Subject: [PATCH 2/7] refactor(vm): making more value member function inline --- .pre-commit-config.yaml | 2 +- include/Ark/VM/Value.hpp | 226 ++++++-------------------------- include/Ark/VM/inline/Value.inl | 75 +---------- src/arkreactor/VM/Value.cpp | 34 +---- 4 files changed, 46 insertions(+), 291 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 917a58701..ceffe09ff 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -14,7 +14,7 @@ repos: rev: '336fdd7c3cab698ead0b1c95157b9e74d3906b62' hooks: - id: cppcheck - args: [ "--platform=unix64", "--inline-suppr", "--suppressions-list=cppcheck-suppressions.txt" ] + args: [ "--platform=unix64", "--language=c++", "--inline-suppr", "--suppressions-list=cppcheck-suppressions.txt" ] - repo: https://github.com/compilerla/conventional-pre-commit rev: 'v3.2.0' hooks: diff --git a/include/Ark/VM/Value.hpp b/include/Ark/VM/Value.hpp index 982641c69..a6a75a664 100644 --- a/include/Ark/VM/Value.hpp +++ b/include/Ark/VM/Value.hpp @@ -2,10 +2,10 @@ * @file Value.hpp * @author Default value type handled by the virtual machine * @brief - * @version 0.3 - * @date 2020-10-27 + * @version 1.0 + * @date 2024-04-20 * - * @copyright Copyright (c) 2020-2021 + * @copyright Copyright (c) 2020-2024 * */ @@ -14,7 +14,7 @@ #include #include -#include // for conversions +#include #include #include #include @@ -30,10 +30,10 @@ namespace Ark { class VM; - // Note from the creator: we can have at most 0b01111111 (127) different types - // because type index is stored on the 7 right most bits of a uint8_t in the class Value. - // Order is also important because we are doing some optimizations to check ranges - // of types based on their integer values. + // Note: we can have at most 0x7f (127) different types + // because type index is stored on the 7 right most bits of a uint8_t in the class Value. + // Order is also important because we are doing some optimizations to check ranges + // of types based on their integer values. enum class ValueType { List = 0, @@ -51,7 +51,7 @@ namespace Ark Reference = 11, InstPtr = 12, - Any = 99 + Any = 99 ///< Used only for typechecking }; const std::array types_to_str = { @@ -108,153 +108,34 @@ namespace Ark m_value(value) {} - /** - * @brief Construct a new Value object as a Number - * - * @param value - */ explicit Value(int value) noexcept; - - /** - * @brief Construct a new Value object as a Number - * - * @param value - */ explicit Value(float value) noexcept; - - /** - * @brief Construct a new Value object as a Number - * - * @param value - */ explicit Value(double value) noexcept; - - /** - * @brief Construct a new Value object as a String - * - * @param value - */ explicit Value(const std::string& value) noexcept; - - /** - * @brief Construct a new Value object as a String - * - * @param value - */ explicit Value(const char* value) noexcept; - - /** - * @brief Construct a new Value object as a Function - * - * @param value - */ explicit Value(internal::PageAddr_t value) noexcept; - - /** - * @brief Construct a new Value object from a C++ function - * - * @param value - */ explicit Value(Value::ProcType value) noexcept; - - /** - * @brief Construct a new Value object as a List - * - * @param value - */ explicit Value(std::vector&& value) noexcept; - - /** - * @brief Construct a new Value object as a Closure - * - * @param value - */ explicit Value(internal::Closure&& value) noexcept; - - /** - * @brief Construct a new Value object as a UserType - * - * @param value - */ explicit Value(UserType&& value) noexcept; - - /** - * @brief Construct a new Value object as a reference to an internal object - * - * @param ref - */ explicit Value(Value* ref) noexcept; - /** - * @brief Return the value type - * - * @return ValueType - */ - inline ValueType valueType() const noexcept; - - /** - * @brief Check if a function is held - * - * @return true on success - * @return false on failure - */ - inline bool isFunction() const noexcept; - - /** - * @brief Return the stored number - * - * @return double - */ - inline double number() const; - - /** - * @brief Return the stored string - * - * @return const std::string& - */ - inline const std::string& string() const; - - /** - * @brief Return the stored list - * - * @return const std::vector& - */ - inline const std::vector& constList() const; - - /** - * @brief Return the stored user type - * - * @return const UserType& - */ - inline const UserType& usertype() const; - - /** - * @brief Return the stored list as a reference - * - * @return std::vector& - */ - std::vector& list(); - - /** - * @brief Return the stored string as a reference - * - * @return std::string& - */ - std::string& stringRef(); - - /** - * @brief Return the stored user type as a reference - * - * @return UserType& - */ - UserType& usertypeRef(); + [[nodiscard]] inline ValueType valueType() const noexcept { return static_cast(type_num()); } + [[nodiscard]] inline bool isFunction() const noexcept + { + auto type = valueType(); + return type == ValueType::PageAddr || type == ValueType::Closure || type == ValueType::CProc || + (type == ValueType::Reference && reference()->isFunction()); + } - /** - * @brief Return the stored internal object reference - * - * @return Value* - */ - Value* reference() const; + [[nodiscard]] inline double number() const { return std::get(m_value); } + [[nodiscard]] inline const std::string& string() const { return std::get(m_value); } + [[nodiscard]] inline const std::vector& constList() const { return std::get>(m_value); } + [[nodiscard]] inline const UserType& usertype() const { return std::get(m_value); } + [[nodiscard]] inline std::vector& list() { return std::get>(m_value); } + [[nodiscard]] inline std::string& stringRef() { return std::get(m_value); } + [[nodiscard]] inline UserType& usertypeRef() { return std::get(m_value); } + [[nodiscard]] inline Value* reference() const { return std::get(m_value); } /** * @brief Add an element to the list held by the value (if the value type is set to list) @@ -282,50 +163,21 @@ namespace Ark uint8_t m_const_type; ///< First bit if for constness, right most bits are for type Value_t m_value; - // private getters only for the virtual machine - - /** - * @brief Return the page address held by the value - * - * @return internal::PageAddr_t - */ - inline internal::PageAddr_t pageAddr() const; - - /** - * @brief Return the C Function held by the value - * - * @return const ProcType& - */ - inline const ProcType& proc() const; - - /** - * @brief Return the closure held by the value - * - * @return const internal::Closure& - */ - inline const internal::Closure& closure() const; - - /** - * @brief Return a reference to the closure held by the value - * - * @return internal::Closure& - */ - internal::Closure& refClosure(); - - /** - * @brief Check if the value is const or not - * - * @return true - * @return false - */ - inline bool isConst() const noexcept; - - /** - * @brief Set the Const object - * - * @param value - */ - inline void setConst(bool value) noexcept; + [[nodiscard]] inline constexpr uint8_t type_num() const noexcept { return m_const_type & 0x7f; } + + [[nodiscard]] inline internal::PageAddr_t pageAddr() const { return std::get(m_value); } + [[nodiscard]] inline const ProcType& proc() const { return std::get(m_value); } + [[nodiscard]] inline const internal::Closure& closure() const { return std::get(m_value); } + [[nodiscard]] inline internal::Closure& refClosure() { return std::get(m_value); } + + [[nodiscard]] inline bool isConst() const noexcept { return m_const_type & (1 << 7); } + inline void setConst(bool value) noexcept + { + if (value) + m_const_type |= 1 << 7; + else + m_const_type = type_num(); + } }; #include "inline/Value.inl" diff --git a/include/Ark/VM/inline/Value.inl b/include/Ark/VM/inline/Value.inl index b8e8ea3a5..b6b9b74ad 100644 --- a/include/Ark/VM/inline/Value.inl +++ b/include/Ark/VM/inline/Value.inl @@ -1,77 +1,10 @@ -// public getters - -inline ValueType Value::valueType() const noexcept -{ - // the type is stored on the right most bits - return static_cast(m_const_type & 0b01111111); -} - -inline bool Value::isFunction() const noexcept // if it's a function we can resolve it -{ - auto type = valueType(); - return type == ValueType::PageAddr || type == ValueType::Closure || type == ValueType::CProc || - (type == ValueType::Reference && reference()->isFunction()); -} - -inline double Value::number() const -{ - return std::get(m_value); -} - -inline const std::string& Value::string() const -{ - return std::get(m_value); -} - -inline const std::vector& Value::constList() const -{ - return std::get>(m_value); -} - -inline const UserType& Value::usertype() const -{ - return std::get(m_value); -} - -// private getters - -inline internal::PageAddr_t Value::pageAddr() const -{ - return std::get(m_value); -} - -inline const Value::ProcType& Value::proc() const -{ - return std::get(m_value); -} - -inline const internal::Closure& Value::closure() const -{ - return std::get(m_value); -} - -inline bool Value::isConst() const noexcept -{ - return m_const_type & (1 << 7); -} - -inline void Value::setConst(bool value) noexcept -{ - if (value) - m_const_type |= 1 << 7; - else - m_const_type &= 0b01111111; // keep only the right most bits -} - -// operators - inline bool operator==(const Value& A, const Value& B) noexcept { // values should have the same type - if (A.valueType() != B.valueType()) + if (A.type_num() != B.type_num()) return false; // all the types >= Nil are Nil itself, True, False, Undefined - else if ((A.m_const_type & 0b01111111) >= static_cast(ValueType::Nil)) + else if (A.type_num() >= static_cast(ValueType::Nil)) return true; return A.m_value == B.m_value; @@ -79,8 +12,8 @@ inline bool operator==(const Value& A, const Value& B) noexcept inline bool operator<(const Value& A, const Value& B) noexcept { - if (A.valueType() != B.valueType()) - return (static_cast(A.valueType()) - static_cast(B.valueType())) < 0; + if (A.type_num() != B.type_num()) + return (A.type_num() - B.type_num()) < 0; return A.m_value < B.m_value; } diff --git a/src/arkreactor/VM/Value.cpp b/src/arkreactor/VM/Value.cpp index 7826c2355..24ab844be 100644 --- a/src/arkreactor/VM/Value.cpp +++ b/src/arkreactor/VM/Value.cpp @@ -63,46 +63,16 @@ namespace Ark m_const_type(init_const_type(true, ValueType::Reference)), m_value(ref) {} - // -------------------------- - - std::vector& Value::list() - { - return std::get>(m_value); - } - - internal::Closure& Value::refClosure() - { - return std::get(m_value); - } - - std::string& Value::stringRef() - { - return std::get(m_value); - } - - UserType& Value::usertypeRef() - { - return std::get(m_value); - } - - Value* Value::reference() const - { - return std::get(m_value); - } - - // -------------------------- - void Value::push_back(const Value& value) { - list().push_back(value); + list().emplace_back(value); } void Value::push_back(Value&& value) { - list().push_back(std::move(value)); + list().emplace_back(std::move(value)); } - void Value::toString(std::ostream& os, VM& vm) const noexcept { switch (valueType()) From abb043b491cab46212cb97c235719df37f8fcf15 Mon Sep 17 00:00:00 2001 From: Alexandre Plateau Date: Sat, 20 Apr 2024 18:25:06 +0200 Subject: [PATCH 3/7] fix(vm): remove unecessary resolveReferenceInPlace inside the VM --- include/Ark/VM/inline/VM.inl | 21 ++------------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/include/Ark/VM/inline/VM.inl b/include/Ark/VM/inline/VM.inl index c6e7934c9..03e3f7ce7 100644 --- a/include/Ark/VM/inline/VM.inl +++ b/include/Ark/VM/inline/VM.inl @@ -1,15 +1,4 @@ -// ------------------------------------------ -// instructions -// ------------------------------------------ - #define resolveRef(valptr) (((valptr)->valueType() == ValueType::Reference) ? *((valptr)->reference()) : *(valptr)) -#define resolveRefInPlace(val) \ - if (val.valueType() == ValueType::Reference) \ - { \ - val.m_const_type = val.reference()->m_const_type; \ - val.m_value = val.reference()->m_value; \ - } - template Value VM::call(const std::string& name, Args&&... args) @@ -197,7 +186,6 @@ inline void VM::swapStackForFunCall(uint16_t argc, internal::ExecutionContext& c case 1: context.stack[context.sp + 1] = context.stack[context.sp - 1]; - resolveRefInPlace(context.stack[context.sp + 1]); context.stack[context.sp - 1] = Value(static_cast(context.pp)); context.stack[context.sp + 0] = Value(ValueType::InstPtr, static_cast(context.ip)); context.sp += 2; @@ -208,19 +196,15 @@ inline void VM::swapStackForFunCall(uint16_t argc, internal::ExecutionContext& c const int16_t first = context.sp - argc; // move first argument to the very end context.stack[context.sp + 1] = context.stack[first + 0]; - resolveRefInPlace(context.stack[context.sp + 1]); - // move second argument right before the last one + // move second argument right before the last one context.stack[context.sp + 0] = context.stack[first + 1]; - resolveRefInPlace(context.stack[context.sp + 0]); - // move the rest, if any + // move the rest, if any int16_t x = 2; const int16_t stop = ((argc % 2 == 0) ? argc : (argc - 1)) / 2; while (x <= stop) { // destination , origin std::swap(context.stack[context.sp - x + 1], context.stack[first + x]); - resolveRefInPlace(context.stack[context.sp - x + 1]); - resolveRefInPlace(context.stack[first + x]); ++x; } context.stack[first + 0] = Value(static_cast(context.pp)); @@ -360,4 +344,3 @@ inline void VM::call(internal::ExecutionContext& context, int16_t argc_) } #undef resolveRef -#undef resolveRefInPlace From ee14f2ad785164cce30ffa40f1c3f63c5cf66386 Mon Sep 17 00:00:00 2001 From: Alexandre Plateau Date: Sat, 20 Apr 2024 18:27:51 +0200 Subject: [PATCH 4/7] refactor(vm): cleaning code --- include/Ark/VM/inline/VM.inl | 2 +- src/arkreactor/VM/VM.cpp | 142 +++--------------------- tests/benchmarks/results/2-abb043b4.csv | 5 + 3 files changed, 24 insertions(+), 125 deletions(-) create mode 100644 tests/benchmarks/results/2-abb043b4.csv diff --git a/include/Ark/VM/inline/VM.inl b/include/Ark/VM/inline/VM.inl index 03e3f7ce7..47179e7c6 100644 --- a/include/Ark/VM/inline/VM.inl +++ b/include/Ark/VM/inline/VM.inl @@ -93,7 +93,7 @@ Value VM::resolve(const Value* val, Args&&... args) inline Value VM::resolve(internal::ExecutionContext* context, std::vector& n) { if (!n[0].isFunction()) - throw TypeError("Value::resolve couldn't resolve a non-function (" + types_to_str[static_cast(n[0].valueType())] + ")"); + throw TypeError("VM::resolve couldn't resolve a non-function (" + types_to_str[static_cast(n[0].valueType())] + ")"); int ip = context->ip; std::size_t pp = context->pp; diff --git a/src/arkreactor/VM/VM.cpp b/src/arkreactor/VM/VM.cpp index 8466ab818..0d0ecb017 100644 --- a/src/arkreactor/VM/VM.cpp +++ b/src/arkreactor/VM/VM.cpp @@ -32,6 +32,12 @@ namespace Ark void VM::init() noexcept { ExecutionContext& context = *m_execution_contexts.back(); + for (auto& c : m_execution_contexts) + { + c->ip = 0; + c->pp = 0; + c->sp = 0; + } context.sp = 0; context.fc = 1; @@ -127,10 +133,20 @@ namespace Ark m_shared_lib_objects.emplace_back(lib); // load the mapping from the dynamic library - mapping* map = nullptr; try { - map = m_shared_lib_objects.back()->template get("getFunctionsMapping")(); + mapping* map = m_shared_lib_objects.back()->template get("getFunctionsMapping")(); + // load the mapping data + std::size_t i = 0; + while (map[i].name != nullptr) + { + // put it in the global frame, aka the first one + auto it = std::find(m_state.m_symbols.begin(), m_state.m_symbols.end(), std::string(map[i].name)); + if (it != m_state.m_symbols.end()) + (context.locals[0]).push_back(static_cast(std::distance(m_state.m_symbols.begin(), it)), Value(map[i].value)); + + ++i; + } } catch (const std::system_error& e) { @@ -139,18 +155,6 @@ namespace Ark "An error occurred while loading module '" + file + "': " + std::string(e.what()) + "\n" + "It is most likely because the versions of the module and the language don't match."); } - - // load the mapping data - std::size_t i = 0; - while (map[i].name != nullptr) - { - // put it in the global frame, aka the first one - auto it = std::find(m_state.m_symbols.begin(), m_state.m_symbols.end(), std::string(map[i].name)); - if (it != m_state.m_symbols.end()) - (context.locals[0]).push_back(static_cast(std::distance(m_state.m_symbols.begin(), it)), Value(map[i].value)); - - ++i; - } } void VM::exit(int code) noexcept @@ -212,22 +216,10 @@ namespace Ark m_futures.erase(it); } - // ------------------------------------------ - // execution - // ------------------------------------------ - int VM::run() noexcept { init(); safeRun(*m_execution_contexts[0]); - - // reset VM after each run - for (auto& context : m_execution_contexts) - { - context->ip = 0; - context->pp = 0; - } - return m_exit_code; } @@ -255,11 +247,6 @@ namespace Ark case Instruction::LOAD_SYMBOL: { - /* - Argument: symbol id (two bytes, big endian) - Job: Load a symbol from its id onto the stack - */ - context.last_symbol = arg; if (Value* var = findNearestVariable(context.last_symbol, context); var != nullptr) @@ -278,12 +265,6 @@ namespace Ark case Instruction::LOAD_CONST: { - /* - Argument: constant id (two bytes, big endian) - Job: Load a constant from its id onto the stack. Should check for a saved environment - and push a Closure with the page address + environment instead of the constant - */ - if (context.saved_scope && m_state.m_constants[arg].valueType() == ValueType::PageAddr) { push(Value(Closure(context.saved_scope.value(), m_state.m_constants[arg].pageAddr())), context); @@ -300,12 +281,6 @@ namespace Ark case Instruction::POP_JUMP_IF_TRUE: { - /* - Argument: absolute address to jump to (two bytes, big endian) - Job: Jump to the provided address if the last value on the stack was equal to true. - Remove the value from the stack no matter what it is - */ - if (*popAndResolveAsPtr(context) == Builtins::trueSym) context.ip = static_cast(arg) * 4; // instructions are 4 bytes break; @@ -313,13 +288,6 @@ namespace Ark case Instruction::STORE: { - /* - Argument: symbol id (two bytes, big endian) - Job: Take the value on top of the stack and put it inside a variable named following - the symbol id (cf symbols table), in the nearest scope. Raise an error if it - couldn't find a scope where the variable exists - */ - if (Value* var = findNearestVariable(arg, context); var != nullptr) { if (var->isConst() && var->valueType() != ValueType::Reference) @@ -341,12 +309,6 @@ namespace Ark case Instruction::LET: { - /* - Argument: symbol id (two bytes, big endian) - Job: Take the value on top of the stack and create a constant in the current scope, named - following the given symbol id (cf symbols table) - */ - // check if we are redefining a variable if (auto val = (context.locals.back())[arg]; val != nullptr) throwVMError(ErrorKind::Mutability, "Can not use 'let' to redefine the variable " + m_state.m_symbols[arg]); @@ -360,12 +322,6 @@ namespace Ark case Instruction::POP_JUMP_IF_FALSE: { - /* - Argument: absolute address to jump to (two bytes, big endian) - Job: Jump to the provided address if the last value on the stack was equal to false. Remove - the value from the stack no matter what it is - */ - if (*popAndResolveAsPtr(context) == Builtins::falseSym) context.ip = static_cast(arg) * 4; // instructions are 4 bytes break; @@ -373,23 +329,12 @@ namespace Ark case Instruction::JUMP: { - /* - Argument: absolute address to jump to (two byte, big endian) - Job: Jump to the provided address - */ - context.ip = static_cast(arg) * 4; // instructions are 4 bytes break; } case Instruction::RET: { - /* - Argument: none - Job: If in a code segment other than the main one, quit it, and push the value on top of - the stack to the new stack ; should as well delete the current environment. - */ - Value ip_or_val = *popAndResolveAsPtr(context); // no return value on the stack if (ip_or_val.valueType() == ValueType::InstPtr) @@ -439,13 +384,6 @@ namespace Ark case Instruction::CAPTURE: { - /* - Argument: symbol id (two bytes, big endian) - Job: Used to tell the Virtual Machine to capture the variable from the current environment. - Main goal is to be able to handle closures, which need to save the environment in which - they were created - */ - if (!context.saved_scope) context.saved_scope = Scope(); @@ -460,23 +398,12 @@ namespace Ark case Instruction::BUILTIN: { - /* - Argument: id of builtin (two bytes, big endian) - Job: Push the builtin function object on the stack - */ - push(Builtins::builtins[arg].second, context); break; } case Instruction::MUT: { - /* - Argument: symbol id (two bytes, big endian) - Job: Take the value on top of the stack and create a variable in the current scope, - named following the given symbol id (cf symbols table) - */ - Value val = *popAndResolveAsPtr(context); val.setConst(false); @@ -492,11 +419,6 @@ namespace Ark case Instruction::DEL: { - /* - Argument: symbol id (two bytes, big endian) - Job: Remove a variable/constant named following the given symbol id (cf symbols table) - */ - if (Value* var = findNearestVariable(arg, context); var != nullptr) { if (var->valueType() == ValueType::User) @@ -511,22 +433,12 @@ namespace Ark case Instruction::SAVE_ENV: { - /* - Argument: none - Job: Save the current environment, useful for quoted code - */ context.saved_scope = context.locals.back(); break; } case Instruction::GET_FIELD: { - /* - Argument: symbol id (two bytes, big endian) - Job: Used to read the field named following the given symbol id (cf symbols table) of a `Closure` - stored in TS. Pop TS and push the value of field read on the stack - */ - Value* var = popAndResolveAsPtr(context); if (var->valueType() != ValueType::Closure) throwVMError(ErrorKind::Type, "The variable `" + m_state.m_symbols[context.last_symbol] + "' isn't a closure, can not get the field `" + m_state.m_symbols[arg] + "' from it"); @@ -548,23 +460,12 @@ namespace Ark case Instruction::PLUGIN: { - /* - Argument: constant id (two bytes, big endian) - Job: Load a module named following the constant id (cf constants table). - Raise an error if it couldn't find the module. - */ - loadPlugin(arg, context); break; } case Instruction::LIST: { - /* - Takes at least 0 arguments and push a list on the stack. - The content is pushed in reverse order - */ - Value l(ValueType::List); if (arg != 0) l.list().reserve(arg); @@ -1120,9 +1021,6 @@ namespace Ark #endif } - if (m_state.m_debug_level > 0) - std::cout << "Estimated stack trashing: " << context.sp << "/" << VMStackSize << "\n"; - #ifdef ARK_PROFILER_MIPS auto end_time = std::chrono::system_clock::now(); auto d = std::chrono::duration_cast(end_time - start_time); @@ -1135,10 +1033,6 @@ namespace Ark return m_exit_code; } - // ------------------------------------------ - // error handling - // ------------------------------------------ - uint16_t VM::findNearestVariableIdWithValue(const Value& value, ExecutionContext& context) const noexcept { for (auto it = context.locals.rbegin(), it_end = context.locals.rend(); it != it_end; ++it) diff --git a/tests/benchmarks/results/2-abb043b4.csv b/tests/benchmarks/results/2-abb043b4.csv new file mode 100644 index 000000000..f2c722815 --- /dev/null +++ b/tests/benchmarks/results/2-abb043b4.csv @@ -0,0 +1,5 @@ +name,iterations,real_time,cpu_time,time_unit,bytes_per_second,items_per_second,label,error_occurred,error_message +"quicksort",4884,0.143883,0.143115,ms,,,,, +"ackermann/iterations:50",50,68.4938,68.1973,ms,,,,, +"fibonacci/iterations:100",100,6.09902,6.07723,ms,,,,, +"man_or_boy",47032,0.0150157,0.0149437,ms,,,,, From 75161de75d72e182ea5d9c39cdef47ee31900d64 Mon Sep 17 00:00:00 2001 From: Alexandre Plateau Date: Sat, 20 Apr 2024 18:32:58 +0200 Subject: [PATCH 5/7] fix(vm): removing unecessary resolveRef inside the VM --- include/Ark/VM/inline/VM.inl | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/include/Ark/VM/inline/VM.inl b/include/Ark/VM/inline/VM.inl index 47179e7c6..8ccbd8732 100644 --- a/include/Ark/VM/inline/VM.inl +++ b/include/Ark/VM/inline/VM.inl @@ -1,5 +1,3 @@ -#define resolveRef(valptr) (((valptr)->valueType() == ValueType::Reference) ? *((valptr)->reference()) : *(valptr)) - template Value VM::call(const std::string& name, Args&&... args) { @@ -67,9 +65,9 @@ Value VM::resolve(const Value* val, Args&&... args) // convert and push arguments in reverse order std::vector fnargs { { Value(std::forward(args))... } }; for (auto it = fnargs.rbegin(), it_end = fnargs.rend(); it != it_end; ++it) - push(resolveRef(it), context); + push(*it, context); // push function - push(resolveRef(val), context); + push(*val, context); std::size_t frames_count = context.fc; // call it @@ -100,7 +98,7 @@ inline Value VM::resolve(internal::ExecutionContext* context, std::vector // convert and push arguments in reverse order for (auto it = n.begin() + 1, it_end = n.end(); it != it_end; ++it) - push(*it, *context); // todo use resolveRef + push(*it, *context); push(n[0], *context); std::size_t frames_count = context->fc; @@ -342,5 +340,3 @@ inline void VM::call(internal::ExecutionContext& context, int16_t argc_) "Function '" + m_state.m_symbols[context.last_symbol] + "' needs " + std::to_string(needed_argc) + " arguments, but it received " + std::to_string(argc)); } - -#undef resolveRef From ad889963f4c84db6f992e94b82da0581e19aa502 Mon Sep 17 00:00:00 2001 From: Alexandre Plateau Date: Sat, 20 Apr 2024 18:55:57 +0200 Subject: [PATCH 6/7] feat(vm): adding likely and unlikely attributes to help the compiler --- include/Ark/VM/inline/VM.inl | 8 ++++---- src/arkreactor/VM/VM.cpp | 27 +++++++++++-------------- tests/benchmarks/compare.py | 2 +- tests/benchmarks/results/3-75161de7.csv | 5 +++++ 4 files changed, 22 insertions(+), 20 deletions(-) create mode 100644 tests/benchmarks/results/3-75161de7.csv diff --git a/include/Ark/VM/inline/VM.inl b/include/Ark/VM/inline/VM.inl index 8ccbd8732..58176aa8b 100644 --- a/include/Ark/VM/inline/VM.inl +++ b/include/Ark/VM/inline/VM.inl @@ -124,7 +124,7 @@ inline Value VM::resolve(internal::ExecutionContext* context, std::vector inline Value* VM::pop(internal::ExecutionContext& context) { - if (context.sp > 0) + if (context.sp > 0) [[likely]] { --context.sp; return &context.stack[context.sp]; @@ -249,7 +249,7 @@ inline void VM::call(internal::ExecutionContext& context, int16_t argc_) uint16_t argc = 0; // handling calls from C++ code - if (argc_ <= -1) + if (argc_ <= -1) [[unlikely]] { ++context.ip; argc = (static_cast(m_state.m_pages[context.pp][context.ip]) << 8) + static_cast(m_state.m_pages[context.pp][context.ip + 1]); @@ -286,7 +286,7 @@ inline void VM::call(internal::ExecutionContext& context, int16_t argc_) swapStackForFunCall(argc, context); // store "reference" to the function to speed the recursive functions - if (context.last_symbol < m_state.m_symbols.size()) + if (context.last_symbol < m_state.m_symbols.size()) [[likely]] context.locals.back().push_back(context.last_symbol, function); context.pp = new_page_pointer; @@ -334,7 +334,7 @@ inline void VM::call(internal::ExecutionContext& context, int16_t argc_) index += 4; // instructions are on 4 bytes } - if (needed_argc != argc) + if (needed_argc != argc) [[unlikely]] throwVMError( ErrorKind::Arity, "Function '" + m_state.m_symbols[context.last_symbol] + "' needs " + std::to_string(needed_argc) + diff --git a/src/arkreactor/VM/VM.cpp b/src/arkreactor/VM/VM.cpp index 0d0ecb017..c806868a3 100644 --- a/src/arkreactor/VM/VM.cpp +++ b/src/arkreactor/VM/VM.cpp @@ -249,7 +249,7 @@ namespace Ark { context.last_symbol = arg; - if (Value* var = findNearestVariable(context.last_symbol, context); var != nullptr) + if (Value* var = findNearestVariable(context.last_symbol, context); var != nullptr) [[likely]] { // push internal reference, shouldn't break anything so far, unless it's already a ref if (var->valueType() == ValueType::Reference) @@ -257,7 +257,7 @@ namespace Ark else push(var, context); } - else + else [[unlikely]] throwVMError(ErrorKind::Scope, "Unbound variable: " + m_state.m_symbols[context.last_symbol]); break; @@ -270,11 +270,8 @@ namespace Ark push(Value(Closure(context.saved_scope.value(), m_state.m_constants[arg].pageAddr())), context); context.saved_scope.reset(); } - else - { - // push internal ref + else [[likely]] // push internal ref push(&(m_state.m_constants[arg]), context); - } break; } @@ -288,14 +285,14 @@ namespace Ark case Instruction::STORE: { - if (Value* var = findNearestVariable(arg, context); var != nullptr) + if (Value* var = findNearestVariable(arg, context); var != nullptr) [[likely]] { if (var->isConst() && var->valueType() != ValueType::Reference) throwVMError(ErrorKind::Mutability, "Can not modify a constant: " + m_state.m_symbols[arg]); if (var->valueType() == ValueType::Reference) *var->reference() = *popAndResolveAsPtr(context); - else + else [[likely]] { *var = *popAndResolveAsPtr(context); var->setConst(false); @@ -310,12 +307,12 @@ namespace Ark case Instruction::LET: { // check if we are redefining a variable - if (auto val = (context.locals.back())[arg]; val != nullptr) + if (auto val = (context.locals.back())[arg]; val != nullptr) [[unlikely]] throwVMError(ErrorKind::Mutability, "Can not use 'let' to redefine the variable " + m_state.m_symbols[arg]); Value val = *popAndResolveAsPtr(context); val.setConst(true); - (context.locals.back()).push_back(arg, val); + context.locals.back().push_back(arg, val); break; } @@ -337,7 +334,7 @@ namespace Ark { Value ip_or_val = *popAndResolveAsPtr(context); // no return value on the stack - if (ip_or_val.valueType() == ValueType::InstPtr) + if (ip_or_val.valueType() == ValueType::InstPtr) [[unlikely]] { context.ip = ip_or_val.pageAddr(); // we always push PP then IP, thus the next value @@ -348,7 +345,7 @@ namespace Ark push(Builtins::nil, context); } // value on the stack - else + else [[likely]] { Value* ip; do @@ -373,7 +370,7 @@ namespace Ark case Instruction::CALL: { // stack pointer + 2 because we push IP and PP - if (context.sp + 2 >= VMStackSize) + if (context.sp + 2 >= VMStackSize) [[unlikely]] throwVMError( ErrorKind::VM, "Maximum recursion depth exceeded.\n" @@ -409,8 +406,8 @@ namespace Ark // avoid adding the pair (id, _) multiple times, with different values Value* local = (context.locals.back())[arg]; - if (local == nullptr) - (context.locals.back()).push_back(arg, val); + if (local == nullptr) [[likely]] + context.locals.back().push_back(arg, val); else *local = val; diff --git a/tests/benchmarks/compare.py b/tests/benchmarks/compare.py index be4b019c5..50ed8cb0f 100644 --- a/tests/benchmarks/compare.py +++ b/tests/benchmarks/compare.py @@ -73,7 +73,7 @@ def diff_from_baseline(self, baseline): dt_real_time=self.real_time - baseline.real_time, dt_rt_percent=(self.real_time - baseline.real_time) / baseline.real_time * 100.0, dt_cpu_time=self.cpu_time - baseline.cpu_time, - dt_ct_percent=(self.cpu_time - baseline.cpu_time) / baseline.cpu_time, + dt_ct_percent=(self.cpu_time - baseline.cpu_time) / baseline.cpu_time * 100.0, time_unit=baseline.time_unit ) diff --git a/tests/benchmarks/results/3-75161de7.csv b/tests/benchmarks/results/3-75161de7.csv new file mode 100644 index 000000000..9c320890b --- /dev/null +++ b/tests/benchmarks/results/3-75161de7.csv @@ -0,0 +1,5 @@ +name,iterations,real_time,cpu_time,time_unit,bytes_per_second,items_per_second,label,error_occurred,error_message +"quicksort",4931,0.143311,0.142738,ms,,,,, +"ackermann/iterations:50",50,68.3526,67.9809,ms,,,,, +"fibonacci/iterations:100",100,6.12407,6.09187,ms,,,,, +"man_or_boy",46336,0.0148826,0.0148246,ms,,,,, From 002d5670b735a3bf8f9d6827ff6ecf58594077c2 Mon Sep 17 00:00:00 2001 From: Alexandre Plateau Date: Sat, 20 Apr 2024 18:57:15 +0200 Subject: [PATCH 7/7] chore(benchmarks): adding the VM benchmarks results --- tests/benchmarks/results/4-ad889963.csv | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 tests/benchmarks/results/4-ad889963.csv diff --git a/tests/benchmarks/results/4-ad889963.csv b/tests/benchmarks/results/4-ad889963.csv new file mode 100644 index 000000000..92e9865af --- /dev/null +++ b/tests/benchmarks/results/4-ad889963.csv @@ -0,0 +1,5 @@ +name,iterations,real_time,cpu_time,time_unit,bytes_per_second,items_per_second,label,error_occurred,error_message +"quicksort",4956,0.141334,0.140946,ms,,,,, +"ackermann/iterations:50",50,67.1292,66.845,ms,,,,, +"fibonacci/iterations:100",100,6.03346,6.01462,ms,,,,, +"man_or_boy",47211,0.0149979,0.0149092,ms,,,,,