Replace mwscript program serialization into a vector with simple struct

Mostly to avoid string literal lookup by index with iteration over all preciding
literals and calling strlen. This is very inefficient. In genral this makes code
much more straightforward but also makes it portable since now int and float of
different sizes are properly supported.

CHANGELOG.md

@@ -41,6 +41,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,25 +105,19 @@ namespace Interpreter
         }
     }
 
-    void Interpreter::run(const Type_Code* code, int codeSize, Context& context)
+    void Interpreter::run(const Program& program, Context& context)
     {
-        assert(codeSize >= 4);
-
         begin();
 
         try
         {
-            mRuntime.configure(code, codeSize, context);
+            mRuntime.configure(program, context);
 
-            int opcodes = static_cast<int>(code[0]);
+            while (mRuntime.getPC() >= 0 && static_cast<std::size_t>(mRuntime.getPC()) < program.mInstructions.size())
-
-            const Type_Code* codeBlock = code + 4;
-
-            while (mRuntime.getPC() >= 0 && mRuntime.getPC() < opcodes)
             {
-                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,12 +7,15 @@
 #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;
@@ -66,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>
@@ -8,58 +9,41 @@ namespace Interpreter
 {
     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 mProgram->mIntegers[static_cast<std::size_t>(index)];
-
-        return *reinterpret_cast<const int*>(&literalBlock[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]);
+        return mProgram->mStrings[static_cast<std::size_t>(index)];
-
-        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;
     }
 
-    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();
     }
 

components/interpreter/runtime.hpp

@@ -9,14 +9,14 @@
 namespace Interpreter
 {
     class Context;
+    struct Program;
 
     /// Runtime data and engine interface
 
     class Runtime
     {
         Context* mContext = nullptr;
-        const Type_Code* mCode = nullptr;
+        const Program* mProgram = nullptr;
-        int mCodeSize = 0;
         int mPC = 0;
         std::vector<Data> mStack;
 
@@ -30,9 +30,9 @@ 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();