Merge branch 'mwscript' into 'master'

Multiple mwscript optimizations and some refactoring

See merge request OpenMW/openmw!2600

CHANGELOG.md

@@ -42,6 +42,7 @@
     Feature #7058: Implement TestModels (T3D) console command
     Feature #7087: Block resolution change in the Windowed Fullscreen mode
     Feature #7130: Ability to set MyGUI logging verbosity
+    Feature #7148: Optimize string literal lookup in mwscript
 
 0.48.0
 ------

apps/openmw/mwdialogue/dialoguemanagerimp.cpp

@@ -2,6 +2,7 @@
 
 #include <algorithm>
 #include <list>
+#include <optional>
 #include <sstream>
 
 #include <components/debug/debuglog.hpp>
@@ -187,10 +188,10 @@ namespace MWDialogue
         return false;
     }
 
-    bool DialogueManager::compile(
+    std::optional<Interpreter::Program> DialogueManager::compile(const std::string& cmd, const MWWorld::Ptr& actor)
-        const std::string& cmd, std::vector<Interpreter::Type_Code>& code, const MWWorld::Ptr& actor)
     {
         bool success = true;
+        std::optional<Interpreter::Program> program;
 
         try
         {
@@ -220,7 +221,7 @@ namespace MWDialogue
                 success = false;
 
             if (success)
-                parser.getCode(code);
+                program = parser.getProgram();
         }
         catch (const Compiler::SourceException& /* error */)
         {
@@ -238,20 +239,19 @@ namespace MWDialogue
             Log(Debug::Error) << "Error: compiling failed (dialogue script): \n" << cmd << "\n";
         }
 
-        return success;
+        return program;
     }
 
     void DialogueManager::executeScript(const std::string& script, const MWWorld::Ptr& actor)
     {
-        std::vector<Interpreter::Type_Code> code;
+        if (const std::optional<Interpreter::Program> program = compile(script, actor))
-        if (compile(script, code, actor))
         {
             try
             {
                 MWScript::InterpreterContext interpreterContext(&actor.getRefData().getLocals(), actor);
                 Interpreter::Interpreter interpreter;
                 MWScript::installOpcodes(interpreter);
-                interpreter.run(code.data(), code.size(), interpreterContext);
+                interpreter.run(*program, interpreterContext);
             }
             catch (const std::exception& error)
             {

apps/openmw/mwdialogue/dialoguemanagerimp.hpp

@@ -4,11 +4,13 @@
 #include "../mwbase/dialoguemanager.hpp"
 
 #include <map>
+#include <optional>
 #include <set>
 #include <unordered_map>
 
 #include <components/compiler/streamerrorhandler.hpp>
 #include <components/esm3/loadinfo.hpp>
+#include <components/interpreter/program.hpp>
 #include <components/misc/strings/algorithm.hpp>
 #include <components/translation/translation.hpp>
 
@@ -63,7 +65,8 @@ namespace MWDialogue
         void updateActorKnownTopics();
         void updateGlobals();
 
-        bool compile(const std::string& cmd, std::vector<Interpreter::Type_Code>& code, const MWWorld::Ptr& actor);
+        std::optional<Interpreter::Program> compile(const std::string& cmd, const MWWorld::Ptr& actor);
+
         void executeScript(const std::string& script, const MWWorld::Ptr& actor);
 
         void executeTopic(const ESM::RefId& topic, ResponseCallback* callback);

apps/openmw/mwgui/console.cpp

@@ -208,9 +208,8 @@ namespace MWGui
                 ConsoleInterpreterContext interpreterContext(*this, mPtr);
                 Interpreter::Interpreter interpreter;
                 MWScript::installOpcodes(interpreter, mConsoleOnlyScripts);
-                std::vector<Interpreter::Type_Code> code;
+                const Interpreter::Program program = output.getProgram();
-                output.getCode(code);
+                interpreter.run(program, interpreterContext);
-                interpreter.run(code.data(), code.size(), interpreterContext);
             }
             catch (const std::exception& error)
             {

apps/openmw/mwscript/scriptmanagerimp.cpp

@@ -79,9 +79,7 @@ namespace MWScript
 
             if (Success)
             {
-                std::vector<Interpreter::Type_Code> code;
+                mScripts.emplace(name, CompiledScript(mParser.getProgram(), mParser.getLocals()));
-                mParser.getCode(code);
-                mScripts.emplace(name, CompiledScript(code, mParser.getLocals()));
 
                 return true;
             }
@@ -100,8 +98,7 @@ namespace MWScript
             if (!compile(name))
             {
                 // failed -> ignore script from now on.
-                std::vector<Interpreter::Type_Code> empty;
+                mScripts.emplace(name, CompiledScript({}, Compiler::Locals()));
-                mScripts.emplace(name, CompiledScript(empty, Compiler::Locals()));
                 return false;
             }
 
@@ -111,7 +108,8 @@ namespace MWScript
 
         // execute script
         const auto& target = interpreterContext.getTarget();
-        if (!iter->second.mByteCode.empty() && iter->second.mInactive.find(target) == iter->second.mInactive.end())
+        if (!iter->second.mProgram.mInstructions.empty()
+            && iter->second.mInactive.find(target) == iter->second.mInactive.end())
             try
             {
                 if (!mOpcodesInstalled)
@@ -120,7 +118,7 @@ namespace MWScript
                     mOpcodesInstalled = true;
                 }
 
-                mInterpreter.run(&iter->second.mByteCode[0], iter->second.mByteCode.size(), interpreterContext);
+                mInterpreter.run(iter->second.mProgram, interpreterContext);
                 return true;
             }
             catch (const MissingImplicitRefError& e)

apps/openmw/mwscript/scriptmanagerimp.hpp

@@ -46,12 +46,12 @@ namespace MWScript
 
         struct CompiledScript
         {
-            std::vector<Interpreter::Type_Code> mByteCode;
+            Interpreter::Program mProgram;
             Compiler::Locals mLocals;
             std::set<ESM::RefId> mInactive;
 
-            CompiledScript(const std::vector<Interpreter::Type_Code>& code, const Compiler::Locals& locals)
+            explicit CompiledScript(Interpreter::Program&& program, const Compiler::Locals& locals)
-                : mByteCode(code)
+                : mProgram(std::move(program))
                 , mLocals(locals)
             {
             }

apps/openmw_test_suite/mwscript/test_scripts.cpp

@@ -21,11 +21,7 @@ namespace
             Compiler::Scanner scanner(mErrorHandler, input, mCompilerContext.getExtensions());
             scanner.scan(mParser);
             if (mErrorHandler.isGood())
-            {
+                return CompiledScript(mParser.getProgram(), mParser.getLocals());
-                std::vector<Interpreter::Type_Code> code;
-                mParser.getCode(code);
-                return CompiledScript(code, mParser.getLocals());
-            }
             else if (!shouldFail)
                 logErrors();
             return {};
@@ -50,7 +46,7 @@ namespace
 
         void run(const CompiledScript& script, TestInterpreterContext& context)
         {
-            mInterpreter.run(&script.mByteCode[0], static_cast<int>(script.mByteCode.size()), context);
+            mInterpreter.run(script.mProgram, context);
         }
 
         template <typename T, typename... TArgs>

apps/openmw_test_suite/mwscript/test_utils.hpp

@@ -249,11 +249,11 @@ namespace
 
     struct CompiledScript
     {
-        std::vector<Interpreter::Type_Code> mByteCode;
+        Interpreter::Program mProgram;
         Compiler::Locals mLocals;
 
-        CompiledScript(const std::vector<Interpreter::Type_Code>& code, const Compiler::Locals& locals)
+        CompiledScript(Interpreter::Program&& program, const Compiler::Locals& locals)
-            : mByteCode(code)
+            : mProgram(std::move(program))
             , mLocals(locals)
         {
         }

components/compiler/fileparser.cpp

@@ -17,9 +17,9 @@ namespace Compiler
         return mName;
     }
 
-    void FileParser::getCode(std::vector<Interpreter::Type_Code>& code) const
+    Interpreter::Program FileParser::getProgram() const
     {
-        mScriptParser.getCode(code);
+        return mScriptParser.getProgram();
     }
 
     const Locals& FileParser::getLocals() const

components/compiler/fileparser.hpp

@@ -31,8 +31,7 @@ namespace Compiler
         std::string getName() const;
         ///< Return script name.
 
-        void getCode(std::vector<Interpreter::Type_Code>& code) const;
+        Interpreter::Program getProgram() const;
-        ///< store generated code in \a code.
 
         const Locals& getLocals() const;
         ///< get local variable declarations.

components/compiler/literals.cpp

@@ -1,56 +1,10 @@
 #include "literals.hpp"
 
 #include <algorithm>
+#include <cstring>
 
 namespace Compiler
 {
-    int Literals::getIntegerSize() const
-    {
-        return static_cast<int>(mIntegers.size() * sizeof(Interpreter::Type_Integer));
-    }
-
-    int Literals::getFloatSize() const
-    {
-        return static_cast<int>(mFloats.size() * sizeof(Interpreter::Type_Float));
-    }
-
-    int Literals::getStringSize() const
-    {
-        int size = 0;
-
-        for (std::vector<std::string>::const_iterator iter(mStrings.begin()); iter != mStrings.end(); ++iter)
-            size += static_cast<int>(iter->size()) + 1;
-
-        if (size % 4) // padding
-            size += 4 - size % 4;
-
-        return size;
-    }
-
-    void Literals::append(std::vector<Interpreter::Type_Code>& code) const
-    {
-        for (const int& mInteger : mIntegers)
-            code.push_back(*reinterpret_cast<const Interpreter::Type_Code*>(&mInteger));
-
-        for (const float& mFloat : mFloats)
-            code.push_back(*reinterpret_cast<const Interpreter::Type_Code*>(&mFloat));
-
-        int stringBlockSize = getStringSize();
-        int size = static_cast<int>(code.size());
-
-        code.resize(size + stringBlockSize / 4);
-
-        size_t offset = 0;
-
-        for (const auto& mString : mStrings)
-        {
-            size_t stringSize = mString.size() + 1;
-
-            std::copy(mString.c_str(), mString.c_str() + stringSize, reinterpret_cast<char*>(&code[size]) + offset);
-            offset += stringSize;
-        }
-    }
-
     int Literals::addInteger(Interpreter::Type_Integer value)
     {
         int index = static_cast<int>(mIntegers.size());

components/compiler/literals.hpp

@@ -17,18 +17,11 @@ namespace Compiler
         std::vector<std::string> mStrings;
 
     public:
-        int getIntegerSize() const;
+        const std::vector<Interpreter::Type_Integer>& getIntegers() const { return mIntegers; }
-        ///< Return size of integer block (in bytes).
 
-        int getFloatSize() const;
+        const std::vector<Interpreter::Type_Float>& getFloats() const { return mFloats; }
-        ///< Return size of float block (in bytes).
 
-        int getStringSize() const;
+        const std::vector<std::string>& getStrings() const { return mStrings; }
-        ///< Return size of string block (in bytes).
-
-        void append(std::vector<Interpreter::Type_Code>& code) const;
-        ///< Apepnd literal blocks to code.
-        /// \note code blocks will be padded for 32-bit alignment.
 
         int addInteger(Interpreter::Type_Integer value);
         ///< add integer liternal and return index.

components/compiler/output.cpp

@@ -13,27 +13,14 @@ namespace Compiler
     {
     }
 
-    void Output::getCode(std::vector<Interpreter::Type_Code>& code) const
+    Interpreter::Program Output::getProgram() const
-    {
+    {
-        code.clear();
+        return Interpreter::Program{
-
+            .mInstructions = mCode,
-        // header
+            .mIntegers = mLiterals.getIntegers(),
-        code.push_back(static_cast<Interpreter::Type_Code>(mCode.size()));
+            .mFloats = mLiterals.getFloats(),
-
+            .mStrings = mLiterals.getStrings(),
-        assert(mLiterals.getIntegerSize() % 4 == 0);
+        };
-        code.push_back(static_cast<Interpreter::Type_Code>(mLiterals.getIntegerSize() / 4));
-
-        assert(mLiterals.getFloatSize() % 4 == 0);
-        code.push_back(static_cast<Interpreter::Type_Code>(mLiterals.getFloatSize() / 4));
-
-        assert(mLiterals.getStringSize() % 4 == 0);
-        code.push_back(static_cast<Interpreter::Type_Code>(mLiterals.getStringSize() / 4));
-
-        // code
-        std::copy(mCode.begin(), mCode.end(), std::back_inserter(code));
-
-        // literals
-        mLiterals.append(code);
     }
 
     const Literals& Output::getLiterals() const

components/compiler/output.hpp

@@ -5,6 +5,7 @@
 
 #include <vector>
 
+#include <components/interpreter/program.hpp>
 #include <components/interpreter/types.hpp>
 
 namespace Compiler
@@ -20,8 +21,7 @@ namespace Compiler
     public:
         Output(Locals& locals);
 
-        void getCode(std::vector<Interpreter::Type_Code>& code) const;
+        Interpreter::Program getProgram() const;
-        ///< store generated code in \a code.
 
         const Literals& getLiterals() const;
 

components/compiler/scriptparser.cpp

@@ -15,9 +15,9 @@ namespace Compiler
     {
     }
 
-    void ScriptParser::getCode(std::vector<Interpreter::Type_Code>& code) const
+    Interpreter::Program ScriptParser::getProgram() const
     {
-        mOutput.getCode(code);
+        return mOutput.getProgram();
     }
 
     bool ScriptParser::parseName(const std::string& name, const TokenLoc& loc, Scanner& scanner)

components/compiler/scriptparser.hpp

@@ -23,8 +23,7 @@ namespace Compiler
         /// \param end of script is marked by end keyword.
         ScriptParser(ErrorHandler& errorHandler, const Context& context, Locals& locals, bool end = false);
 
-        void getCode(std::vector<Interpreter::Type_Code>& code) const;
+        Interpreter::Program getProgram() const;
-        ///< store generated code in \a code.
 
         bool parseName(const std::string& name, const TokenLoc& loc, Scanner& scanner) override;
         ///< Handle a name token.

components/interpreter/interpreter.cpp

@@ -5,6 +5,7 @@
 #include <string>
 
 #include "opcodes.hpp"
+#include "program.hpp"
 
 namespace Interpreter
 {
@@ -104,30 +105,19 @@ namespace Interpreter
         }
     }
 
-    Interpreter::Interpreter()
+    void Interpreter::run(const Program& program, Context& context)
-        : mRunning(false)
     {
-    }
-
-    void Interpreter::run(const Type_Code* code, int codeSize, Context& context)
-    {
-        assert(codeSize >= 4);
-
         begin();
 
         try
         {
-            mRuntime.configure(code, codeSize, context);
+            mRuntime.configure(program, context);
-
-            int opcodes = static_cast<int>(code[0]);
-
-            const Type_Code* codeBlock = code + 4;
 
-            while (mRuntime.getPC() >= 0 && mRuntime.getPC() < opcodes)
+            while (mRuntime.getPC() >= 0 && static_cast<std::size_t>(mRuntime.getPC()) < program.mInstructions.size())
             {
-                Type_Code runCode = codeBlock[mRuntime.getPC()];
+                const Type_Code instruction = program.mInstructions[mRuntime.getPC()];
                 mRuntime.setPC(mRuntime.getPC() + 1);
-                execute(runCode);
+                execute(instruction);
             }
         }
         catch (...)

components/interpreter/interpreter.hpp

@@ -7,26 +7,25 @@
 #include <stack>
 #include <utility>
 
+#include "components/interpreter/program.hpp"
 #include "opcodes.hpp"
 #include "runtime.hpp"
 #include "types.hpp"
 
 namespace Interpreter
 {
+    struct Program;
+
     class Interpreter
     {
         std::stack<Runtime> mCallstack;
-        bool mRunning;
+        bool mRunning = false;
         Runtime mRuntime;
         std::map<int, std::unique_ptr<Opcode1>> mSegment0;
         std::map<int, std::unique_ptr<Opcode1>> mSegment2;
         std::map<int, std::unique_ptr<Opcode1>> mSegment3;
         std::map<int, std::unique_ptr<Opcode0>> mSegment5;
 
-        // not implemented
-        Interpreter(const Interpreter&);
-        Interpreter& operator=(const Interpreter&);
-
         void execute(Type_Code code);
 
         void begin();
@@ -41,7 +40,10 @@ namespace Interpreter
         }
 
     public:
-        Interpreter();
+        Interpreter() = default;
+
+        Interpreter(const Interpreter&) = delete;
+        Interpreter& operator=(const Interpreter&) = delete;
 
         template <typename T, typename... TArgs>
         void installSegment0(int code, TArgs&&... args)
@@ -67,7 +69,7 @@ namespace Interpreter
             installSegment(mSegment5, code, std::make_unique<T>(std::forward<TArgs>(args)...));
         }
 
-        void run(const Type_Code* code, int codeSize, Context& context);
+        void run(const Program& program, Context& context);
     };
 }
 

components/interpreter/program.hpp

@@ -0,0 +1,20 @@
+#ifndef OPENMW_COMPONENTS_INTERPRETER_PROGRAM_H
+#define OPENMW_COMPONENTS_INTERPRETER_PROGRAM_H
+
+#include "types.hpp"
+
+#include <string>
+#include <vector>
+
+namespace Interpreter
+{
+    struct Program
+    {
+        std::vector<Type_Code> mInstructions;
+        std::vector<Type_Integer> mIntegers;
+        std::vector<Type_Float> mFloats;
+        std::vector<std::string> mStrings;
+    };
+}
+
+#endif

components/interpreter/runtime.cpp

@@ -1,4 +1,5 @@
 #include "runtime.hpp"
+#include "program.hpp"
 
 #include <cassert>
 #include <cstring>
@@ -6,81 +7,46 @@
 
 namespace Interpreter
 {
-    Runtime::Runtime()
-        : mContext(nullptr)
-        , mCode(nullptr)
-        , mCodeSize(0)
-        , mPC(0)
-    {
-    }
-
-    int Runtime::getPC() const
-    {
-        return mPC;
-    }
-
     int Runtime::getIntegerLiteral(int index) const
     {
-        if (index < 0 || index >= static_cast<int>(mCode[1]))
+        if (index < 0 || mProgram->mIntegers.size() <= static_cast<std::size_t>(index))
-            throw std::out_of_range("out of range");
+            throw std::out_of_range("Invalid integer index");
-
-        const Type_Code* literalBlock = mCode + 4 + mCode[0];
 
-        return *reinterpret_cast<const int*>(&literalBlock[index]);
+        return mProgram->mIntegers[static_cast<std::size_t>(index)];
     }
 
     float Runtime::getFloatLiteral(int index) const
     {
-        if (index < 0 || index >= static_cast<int>(mCode[2]))
+        if (index < 0 || mProgram->mFloats.size() <= static_cast<std::size_t>(index))
-            throw std::out_of_range("out of range");
+            throw std::out_of_range("Invalid float index");
 
-        const Type_Code* literalBlock = mCode + 4 + mCode[0] + mCode[1];
+        return mProgram->mFloats[static_cast<std::size_t>(index)];
-
-        return *reinterpret_cast<const float*>(&literalBlock[index]);
     }
 
     std::string_view Runtime::getStringLiteral(int index) const
     {
-        if (index < 0 || static_cast<int>(mCode[3]) <= 0)
+        if (index < 0 || mProgram->mStrings.size() <= static_cast<std::size_t>(index))
-            throw std::out_of_range("out of range");
+            throw std::out_of_range("Invalid string literal index");
-
-        const char* literalBlock = reinterpret_cast<const char*>(mCode + 4 + mCode[0] + mCode[1] + mCode[2]);
-
-        size_t offset = 0;
-
-        for (; index; --index)
-        {
-            offset += std::strlen(literalBlock + offset) + 1;
-            if (offset / 4 >= mCode[3])
-                throw std::out_of_range("out of range");
-        }
 
-        return literalBlock + offset;
+        return mProgram->mStrings[static_cast<std::size_t>(index)];
     }
 
-    void Runtime::configure(const Type_Code* code, int codeSize, Context& context)
+    void Runtime::configure(const Program& program, Context& context)
     {
         clear();
 
         mContext = &context;
-        mCode = code;
+        mProgram = &program;
-        mCodeSize = codeSize;
         mPC = 0;
     }
 
     void Runtime::clear()
     {
         mContext = nullptr;
-        mCode = nullptr;
+        mProgram = nullptr;
-        mCodeSize = 0;
         mStack.clear();
     }
 
-    void Runtime::setPC(int PC)
-    {
-        mPC = PC;
-    }
-
     void Runtime::push(const Data& data)
     {
         mStack.push_back(data);
@@ -108,12 +74,12 @@ namespace Interpreter
         mStack.pop_back();
     }
 
-    Data& Runtime::operator[](int Index)
+    Data& Runtime::operator[](int index)
     {
-        if (Index < 0 || Index >= static_cast<int>(mStack.size()))
+        if (index < 0 || index >= static_cast<int>(mStack.size()))
             throw std::runtime_error("stack index out of range");
 
-        return mStack[mStack.size() - Index - 1];
+        return mStack[mStack.size() - index - 1];
     }
 
     Context& Runtime::getContext()

components/interpreter/runtime.hpp

@@ -9,21 +9,19 @@
 namespace Interpreter
 {
     class Context;
+    struct Program;
 
     /// Runtime data and engine interface
 
     class Runtime
     {
-        Context* mContext;
+        Context* mContext = nullptr;
-        const Type_Code* mCode;
+        const Program* mProgram = nullptr;
-        int mCodeSize;
+        int mPC = 0;
-        int mPC;
         std::vector<Data> mStack;
 
     public:
-        Runtime();
+        int getPC() const { return mPC; }
-
-        int getPC() const;
         ///< return program counter.
 
         int getIntegerLiteral(int index) const;
@@ -32,13 +30,13 @@ namespace Interpreter
 
         std::string_view getStringLiteral(int index) const;
 
-        void configure(const Type_Code* code, int codeSize, Context& context);
+        void configure(const Program& program, Context& context);
         ///< \a context and \a code must exist as least until either configure, clear or
-        /// the destructor is called. \a codeSize is given in 32-bit words.
+        /// the destructor is called.
 
         void clear();
 
-        void setPC(int PC);
+        void setPC(int value) { mPC = value; }
         ///< set program counter.
 
         void push(const Data& data);
@@ -53,7 +51,7 @@ namespace Interpreter
         void pop();
         ///< pop stack
 
-        Data& operator[](int Index);
+        Data& operator[](int index);
         ///< Access stack member, counted from the top.
 
         Context& getContext();

components/interpreter/types.hpp

@@ -5,15 +5,13 @@
 
 namespace Interpreter
 {
-    typedef unsigned int Type_Code; // 32 bit
+    typedef std::uint32_t Type_Code;
 
-    typedef unsigned int Type_Data; // 32 bit
+    typedef std::int16_t Type_Short;
 
-    typedef short Type_Short; // 16 bit
+    typedef std::int32_t Type_Integer;
 
-    typedef int Type_Integer; // 32 bit
+    typedef float Type_Float;
-
-    typedef float Type_Float; // 32 bit
 
     union Data
     {