Merge pull request #2187 from ivan-mogilko/361--scriptstringperf5

Made ScriptString store length in data header, use to speed up API functions somewhat

Engine/ac/audioclip.cpp

@@ -104,8 +104,6 @@ ScriptAudioChannel* AudioClip_PlayOnChannel(ScriptAudioClip *clip, int chan, int
 #include "script/script_api.h"
 #include "script/script_runtime.h"
 
-extern ScriptString myScriptStringImpl;
-
 ScriptAudioClip *AudioClip_GetByName(const char *name)
 {
     return static_cast<ScriptAudioClip*>(ccGetScriptObjectAddress(name, ccDynamicAudioClip.GetType()));

Engine/ac/button.cpp

@@ -320,8 +320,6 @@ void Button_SetTextAlignment(GUIButton *butt, int align)
 #include "script/script_runtime.h"
 #include "ac/dynobj/scriptstring.h"
 
-extern ScriptString myScriptStringImpl;
-
 // void | GUIButton *butt, int view, int loop, int speed, int repeat
 RuntimeScriptValue Sc_Button_Animate4(void *self, const RuntimeScriptValue *params, int32_t param_count)
 {

Engine/ac/character.cpp

@@ -3030,8 +3030,6 @@ PViewport FindNearestViewport(int charid)
 #include "script/script_runtime.h"
 #include "ac/dynobj/scriptstring.h"
 
-extern ScriptString myScriptStringImpl;
-
 CharacterInfo *Character_GetByName(const char *name)
 {
     return static_cast<CharacterInfo*>(ccGetScriptObjectAddress(name, ccDynamicCharacter.GetType()));

Engine/ac/dialog.cpp

@@ -1330,7 +1330,6 @@ void do_conversation(int dlgnum)
 #include "ac/dynobj/cc_dialog.h"
 #include "ac/dynobj/scriptstring.h"
 
-extern ScriptString myScriptStringImpl;
 extern CCDialog     ccDynamicDialog;
 
 ScriptDialog *Dialog_GetByName(const char *name)

Engine/ac/dynobj/cc_dynamicarray.cpp

@@ -14,7 +14,7 @@
 #include "cc_dynamicarray.h"
 #include <string.h>
 #include "ac/dynobj/dynobj_manager.h"
-#include "util/memorystream.h"
+#include "ac/dynobj/scriptstring.h"
 
 using namespace AGS::Common;
 
@@ -58,7 +58,7 @@ size_t CCDynamicArray::CalcSerializeSize(const void *address)
     return hdr.TotalSize + FileHeaderSz;
 }
 
-void CCDynamicArray::Serialize(const void *address, AGS::Common::Stream *out)
+void CCDynamicArray::Serialize(const void *address, Stream *out)
 {
     const Header &hdr = GetHeader(address);
     out->WriteInt32(hdr.ElemCount);
@@ -107,7 +107,7 @@ DynObjectRef DynamicArrayHelpers::CreateStringArray(const std::vector<const char
     int32_t *slots = static_cast<int32_t*>(arr.Obj);
     for (auto s : items)
     {
-        DynObjectRef str = stringClassImpl->CreateString(s);
+        DynObjectRef str = ScriptString::Create(s);
         // We must add reference count, because the string is going to be saved
         // within another object (array), not returned to script directly
         ccAddObjectReference(str.Handle);

Engine/ac/dynobj/cc_scriptobject.h

@@ -38,6 +38,7 @@ struct DynObjectRef
     DynObjectRef() = default;
     DynObjectRef(int handle, void *obj, IScriptObject *mgr)
         : Handle(handle), Obj(obj), Mgr(mgr) {}
+    inline operator bool() const { return Handle > 0; }
 };
 
 
@@ -105,10 +106,4 @@ struct ICCObjectReader
     virtual void Unserialize(int32_t handle, const char *serializedData, int dataSize) = 0;
 };
 
-// The interface of a dynamic String allocator.
-struct ICCStringClass
-{
-    virtual DynObjectRef CreateString(const char *fromText) = 0;
-};
-
 #endif // __CC_SCRIPTOBJECT_H

Engine/ac/dynobj/cc_serializer.cpp

@@ -98,8 +98,7 @@ void AGSDeSerializer::Unserialize(int index, const char *objectType, const char
         ccDynamicObject.Unserialize(index, &mems, data_sz);
     }
     else if (strcmp(objectType, "String") == 0) {
-        ScriptString *scf = new ScriptString();
-        scf->Unserialize(index, &mems, data_sz);
+        myScriptStringImpl.Unserialize(index, &mems, data_sz);
     }
     else if (strcmp(objectType, "File") == 0) {
         // files cannot be restored properly -- so just recreate

Engine/ac/dynobj/dynobj_manager.cpp

@@ -21,13 +21,6 @@
 
 using namespace AGS::Common;
 
-ICCStringClass *stringClassImpl = nullptr;
-
-// set the class that will be used for dynamic strings
-void ccSetStringClassImpl(ICCStringClass *theClass) {
-    stringClassImpl = theClass;
-}
-
 // register a memory handle for the object and allow script
 // pointers to point to it
 int32_t ccRegisterManagedObject(void *object, IScriptObject *callback, ScriptValueType obj_type) {

Engine/ac/dynobj/dynobj_manager.h

@@ -28,8 +28,6 @@
 namespace AGS { namespace Common { class Stream; } }
 using namespace AGS; // FIXME later
 
-// set the class that will be used for dynamic strings
-extern void  ccSetStringClassImpl(ICCStringClass *theClass);
 // register a memory handle for the object and allow script
 // pointers to point to it
 extern int32_t ccRegisterManagedObject(void *object, IScriptObject *, ScriptValueType obj_type = kScValScriptObject);
@@ -53,6 +51,4 @@ extern ScriptValueType ccGetObjectAddressAndManagerFromHandle(int32_t handle, vo
 extern int ccAddObjectReference(int32_t handle);
 extern int ccReleaseObjectReference(int32_t handle);
 
-extern ICCStringClass *stringClassImpl;
-
 #endif // __AGS_EE_DYNOBJ__DYNOBJMANAGER_H

Engine/ac/dynobj/scriptstring.cpp

@@ -14,65 +14,92 @@
 #include "ac/dynobj/scriptstring.h"
 #include <stdlib.h>
 #include <string.h>
+#include <allegro.h>
 #include "ac/string.h"
 #include "ac/dynobj/dynobj_manager.h"
 #include "util/stream.h"
 
 using namespace AGS::Common;
 
+ScriptString myScriptStringImpl;
 
-DynObjectRef ScriptString::CreateString(const char *fromText) {
-    return CreateNewScriptStringObj(fromText);
+const char *ScriptString::GetType()
+{
+    return "String";
 }
 
-int ScriptString::Dispose(void* /*address*/, bool /*force*/) {
-    // always dispose
-    if (_text) {
-        free(_text);
-        _text = nullptr;
-    }
-    delete this;
+int ScriptString::Dispose(void *address, bool /*force*/)
+{
+    delete[] (static_cast<uint8_t*>(address) - MemHeaderSz);
     return 1;
 }
 
-const char *ScriptString::GetType() {
-    return "String";
+size_t ScriptString::CalcSerializeSize(const void *address)
+{
+    const Header &hdr = GetHeader(address);
+    return hdr.Length + 1 + FileHeaderSz;
 }
 
-size_t ScriptString::CalcSerializeSize(const void* /*address*/)
+void ScriptString::Serialize(const void* address, Stream *out)
 {
-    return _len + 1 + sizeof(int32_t);
+    const Header &hdr = GetHeader(address);
+    out->WriteInt32(hdr.Length);
+    out->Write(address, hdr.Length + 1); // it was writing trailing 0 for some reason
 }
 
-void ScriptString::Serialize(const void* /*address*/, Stream *out) {
-    const auto *cstr = _text ? _text : "";
-    out->WriteInt32(_len);
-    out->Write(cstr, _len + 1);
+void ScriptString::Unserialize(int index, Stream *in, size_t /*data_sz*/)
+{
+    size_t len = in->ReadInt32();
+    uint8_t *buf = new uint8_t[len + 1 + MemHeaderSz];
+    char *text_ptr = reinterpret_cast<char*>(buf + MemHeaderSz);
+    in->Read(text_ptr, len + 1); // it was writing trailing 0 for some reason
+    text_ptr[len] = 0; // for safety
+    Header &hdr = reinterpret_cast<Header&>(*buf);
+    hdr.Length = len;
+    hdr.ULength = ustrlen(text_ptr);
+    ccRegisterUnserializedObject(index, text_ptr, this);
 }
 
-void ScriptString::Unserialize(int index, Stream *in, size_t /*data_sz*/) {
-    _len = in->ReadInt32();
-    _text = (char*)malloc(_len + 1);
-    in->Read(_text, _len + 1);
-    _text[_len] = 0; // for safety
-    ccRegisterUnserializedObject(index, _text, this);
+DynObjectRef ScriptString::CreateObject(uint8_t *buf)
+{
+    char *text_ptr = reinterpret_cast<char*>(buf + MemHeaderSz);
+    int32_t handle = ccRegisterManagedObject(text_ptr, &myScriptStringImpl);
+    if (handle == 0)
+    {
+        delete[] buf;
+        return DynObjectRef();
+    }
+    return DynObjectRef(handle, text_ptr, &myScriptStringImpl);
 }
 
-ScriptString::ScriptString(const char *text) {
-    _len = strlen(text);
-    _text = (char*)malloc(_len + 1);
-    memcpy(_text, text, _len + 1);
+ScriptString::Buffer ScriptString::CreateBuffer(size_t len, size_t ulen)
+{
+    assert(ulen <= len);
+    std::unique_ptr<uint8_t[]> buf(new uint8_t[len + 1 + MemHeaderSz]);
+    auto *header = reinterpret_cast<Header*>(buf.get());
+    header->Length = len;
+    header->ULength = ulen;
+    header->LastCharIdx = 0;
+    header->LastCharOff = 0;
+    return Buffer(std::move(buf), len + 1 + MemHeaderSz);
 }
 
-ScriptString::ScriptString(char *text, bool take_ownership) {
-    _len = strlen(text);
-    if (take_ownership)
-    {
-        _text = text;
-    }
-    else
-    {
-        _text = (char*)malloc(_len + 1);
-        memcpy(_text, text, _len + 1);
-    }
+DynObjectRef ScriptString::Create(const char *text)
+{
+    int len, ulen;
+    ustrlen2(text, &len, &ulen);
+    auto buf = CreateBuffer(len, ulen);
+    memcpy(buf.Get(), text, len + 1);
+    return CreateObject(buf._buf.release());
+}
+
+DynObjectRef ScriptString::Create(Buffer &&strbuf)
+{
+    uint8_t *buf = strbuf._buf.release();
+    auto *header = reinterpret_cast<Header*>(buf);
+    char *text_ptr = reinterpret_cast<char*>(buf + MemHeaderSz);
+    if ((header->Length > 0) && (header->ULength == 0u))
+        header->ULength = ustrlen(text_ptr);
+    text_ptr[header->Length] = 0; // for safety
+    return CreateObject(buf);
 }

Engine/ac/dynobj/scriptstring.h

@@ -14,31 +14,89 @@
 #ifndef __AC_SCRIPTSTRING_H
 #define __AC_SCRIPTSTRING_H
 
+#include <memory>
 #include "ac/dynobj/cc_agsdynamicobject.h"
 
-struct ScriptString final : AGSCCDynamicObject, ICCStringClass {
-    int Dispose(void *address, bool force) override;
+struct ScriptString final : AGSCCDynamicObject
+{
+public:
+    struct Header
+    {
+        uint32_t Length = 0u;  // string length in bytes (not counting 0)
+        uint32_t ULength = 0u; // Unicode compatible length in characters
+        // Saved last requested character index and buffer offset;
+        // significantly speeds up Unicode string iteration, but adds 4 bytes
+        // per ScriptString object. Replace with a proper str iterator later!
+        // NOTE: intentionally limited to 64k chars/bytes to save bit of mem.
+        uint16_t LastCharIdx = 0u;
+        uint16_t LastCharOff = 0u;
+    };
+
+    struct Buffer
+    {
+        friend ScriptString;
+    public:
+        Buffer() = default;
+        ~Buffer() = default;
+        Buffer(Buffer &&buf) = default;
+        // Returns a pointer to the beginning of a text buffer
+        char *Get() { return reinterpret_cast<char*>(_buf.get() + MemHeaderSz); }
+        // Returns size allocated for a text content (includes null pointer)
+        size_t GetSize() const { return _sz - MemHeaderSz; }
+
+    private:
+        Buffer(std::unique_ptr<uint8_t[]> &&buf, size_t buf_sz)
+            : _buf(std::move(buf)), _sz(buf_sz) {}
+
+        std::unique_ptr<uint8_t[]> _buf;
+        size_t _sz;
+    };
+
+
+    ScriptString() = default;
+    ~ScriptString() = default;
+
+    inline static const Header &GetHeader(const void *address)
+    {
+        return reinterpret_cast<const Header&>(*(static_cast<const uint8_t*>(address) - MemHeaderSz));
+    }
+
+    inline static Header &GetHeader(void *address)
+    {
+        return reinterpret_cast<Header&>(*(static_cast<uint8_t*>(address) - MemHeaderSz));
+    }
+
+    // Allocates a ScriptString-compatible buffer large enough to accomodate
+    // given length in bytes (len). This buffer may be filled by the caller
+    // and then passed into Create(). If ulen is left eq 0, then it will be
+    // recalculated on script string's creation.
+    static Buffer CreateBuffer(size_t len, size_t ulen = 0u);
+    // Create a new script string by copying the given text
+    static DynObjectRef Create(const char *text);
+    // Create a new script string by taking ownership over the given buffer;
+    // passed buffer variable becomes invalid after this call.
+    static DynObjectRef Create(Buffer &&strbuf);
+
     const char *GetType() override;
+    int Dispose(void *address, bool force) override;
     void Unserialize(int index, AGS::Common::Stream *in, size_t data_sz) override;
 
-    DynObjectRef CreateString(const char *fromText) override;
+private:
+    friend ScriptString::Buffer;
+    // The size of the array's header in memory, prepended to the element data
+    static const size_t MemHeaderSz = sizeof(Header);
+    // The size of the serialized header
+    static const size_t FileHeaderSz = sizeof(uint32_t);
 
-    ScriptString() = default;
-    ScriptString(const char *text);
-    ScriptString(char *text, bool take_ownership);
-    char *GetTextPtr() const { return _text; }
+    static DynObjectRef CreateObject(uint8_t *buf);
 
-protected:
+    // Savegame serialization
     // Calculate and return required space for serialization, in bytes
     size_t CalcSerializeSize(const void *address) override;
     // Write object data into the provided stream
     void Serialize(const void *address, AGS::Common::Stream *out) override;
-
-private:
-    // TODO: the preallocated text buffer may be assigned externally;
-    // find out if it's possible to refactor while keeping same functionality
-    char *_text = nullptr;
-    size_t _len = 0;
 };
 
-#endif // __AC_SCRIPTSTRING_H
+extern ScriptString myScriptStringImpl;
+
+#endif // __AC_SCRIPTSTRING_H

Engine/ac/file.cpp

@@ -166,17 +166,16 @@ const char* File_ReadStringBack(sc_File *fil) {
     return CreateNewScriptString("");
   }
 
-  size_t lle = (uint32_t)in->ReadInt32();
-  if (lle == 0)
+  size_t data_sz = (uint32_t)in->ReadInt32();
+  if (data_sz == 0)
   {
     debug_script_warn("File.ReadStringBack: file was not written by WriteString");
     return CreateNewScriptString("");;
   }
 
-  char *retVal = (char*)malloc(lle);
-  in->Read(retVal, lle);
-
-  return CreateNewScriptString(retVal, false);
+  auto buf = ScriptString::CreateBuffer(data_sz - 1); // stored len + 1
+  in->Read(buf.Get(), data_sz);
+  return CreateNewScriptString(std::move(buf));
 }
 
 int File_ReadInt(sc_File *fil) {
@@ -726,7 +725,6 @@ Stream *get_valid_file_stream_from_handle(int32_t handle, const char *operation_
 #include "script/script_runtime.h"
 #include "ac/dynobj/scriptstring.h"
 
-extern ScriptString myScriptStringImpl;
 
 // int (const char *fnmm)
 RuntimeScriptValue Sc_File_Delete(const RuntimeScriptValue *params, int32_t param_count)

Engine/ac/game.cpp

@@ -128,7 +128,6 @@ CCObject    ccDynamicObject;
 CCDialog    ccDynamicDialog;
 CCAudioClip ccDynamicAudioClip;
 CCAudioChannel ccDynamicAudio;
-ScriptString myScriptStringImpl;
 ScriptObject scrObj[MAX_ROOM_OBJECTS];
 std::vector<ScriptGUI> scrGui;
 ScriptHotspot scrHotspot[MAX_ROOM_HOTSPOTS];