From 3913091a021a8f4525f0050dfb83d1c2b8fc6f6b Mon Sep 17 00:00:00 2001 From: jsmall-nvidia Date: Thu, 8 Jun 2023 17:26:33 -0400 Subject: Improvements around StringBlob (#2921) * #include an absolute path didn't work - because paths were taken to always be relative. * Small fixes and improvements around reflection tool. * Make PrettyWriter printing a class. * Improvements around handling StringBlob and storing stdlib source in ISlangBlob. * Fix some issues with comments around StringBlob. * Default initialize StringBlob fields. --- source/core/slang-blob.cpp | 122 +++++++++++++++++++++++++++++++++++++-------- source/core/slang-blob.h | 45 +++++++++-------- source/core/slang-string.h | 4 ++ 3 files changed, 128 insertions(+), 43 deletions(-) (limited to 'source/core') diff --git a/source/core/slang-blob.cpp b/source/core/slang-blob.cpp index a4acb98cb..c360f483b 100644 --- a/source/core/slang-blob.cpp +++ b/source/core/slang-blob.cpp @@ -35,25 +35,115 @@ void* BlobBase::castAs(const SlangUUID& guid) /* !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! StringBlob !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! */ -StringBlob::StringBlob(MoveUnique, String& in) +void StringBlob::_uniqueInit(StringRepresentation* uniqueRep) { - auto rep = in.getStringRepresentation(); - if (rep && !rep->isUniquelyReferenced()) + // When initing the rep either has to be nullptr *or* uniquely referenced + // so we can take ownership of it. + + SLANG_ASSERT(uniqueRep == nullptr || uniqueRep->isUniquelyReferenced()); + + m_rep = uniqueRep; + + // If it's nullptr, that means the string is the empty string. To handle we + // + if (!uniqueRep) + { + m_chars = ""; + m_charsCount = 0; + } + else { - // Make a new unique copy - m_string = in.getUnownedSlice(); + m_chars = uniqueRep->getData(); + m_charsCount = uniqueRep->getLength(); + } + + _checkRep(); +} - // Move out of in - String tmp; - tmp.swapWith(in); +void StringBlob::_init(StringRepresentation* rep) +{ + if (rep) + { + // Even if the uniqueRep is 1, we can't assume we can take ownership + // So we make a copy + + // If the length is 0, we can just init as empty + auto length = rep->getLength(); + if (length == 0) + { + rep = nullptr; + } + else + { + const UnownedStringSlice slice(rep->getData(), length); + rep = StringRepresentation::createWithReference(slice); + } + } + + // Must be unique at this point + _uniqueInit(rep); + + _checkRep(); +} + +void StringBlob::_moveInit(StringRepresentation* rep) +{ + if (rep && !rep->isUniquelyReferenced()) + { + // Will make a copy of the rep + _init(rep); + // We need to release a ref as rep is passed in with the 'current' ref count + rep->releaseReference(); } else { - // Must either not have a rep or be unique - m_string.swapWith(in); + _uniqueInit(rep); + } + _checkRep(); +} + +/* static */ComPtr StringBlob::create(const UnownedStringSlice& slice) +{ + StringRepresentation* rep = nullptr; + if (slice.getLength()) + { + rep = StringRepresentation::createWithReference(slice); } + + // rep must be unique at this point + auto blob = new StringBlob; + blob->_uniqueInit(rep); + return ComPtr(blob); +} + +/* static */ComPtr StringBlob::create(const String& in) +{ + auto blob = new StringBlob; + blob->_init(in.getStringRepresentation()); + return ComPtr(blob); } +/* static */ComPtr StringBlob::moveCreate(String& in) +{ + auto blob = new StringBlob; + blob->_moveInit(in.detachStringRepresentation()); + return ComPtr(blob); +} + +/* static */ComPtr StringBlob::moveCreate(String&& in) +{ + auto blob = new StringBlob; + blob->_moveInit(in.detachStringRepresentation()); + return ComPtr(blob); +} + +StringBlob::~StringBlob() +{ + if (m_rep) + { + delete m_rep; + } +} void* StringBlob::castAs(const SlangUUID& guid) { if (auto intf = getInterface(guid)) @@ -73,21 +163,11 @@ void* StringBlob::getObject(const Guid& guid) // Can always be accessed as terminated char* if (guid == SlangTerminatedChars::getTypeGuid()) { - return const_cast(m_string.getBuffer()); + return const_cast(m_chars); } return nullptr; } -/* static */ComPtr StringBlob::moveCreate(String& in) -{ - return ComPtr(new StringBlob(MoveUnique{}, in)); -} - -/* static */ComPtr StringBlob::moveCreate(String&& in) -{ - return ComPtr(new StringBlob(MoveUnique{}, in)); -} - /* !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! RawBlob !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! */ void* RawBlob::castAs(const SlangUUID& guid) diff --git a/source/core/slang-blob.h b/source/core/slang-blob.h index aa503a6e5..f439a4eae 100644 --- a/source/core/slang-blob.h +++ b/source/core/slang-blob.h @@ -31,11 +31,10 @@ protected: void* getObject(const Guid& guid); }; -/** A blob that uses a `String` for its storage. -NOTE! Returns length *WITHOUT* terminating 0, even though there is one. +/** A blob that uses a `StringRepresentation` for its storage. -NOTE! Whilst BobBase is atomic ref counted, the contained string *is not*. -There is a reasonable argument that StringBlob should contain it's own copy of the string contents. +By design the StringBlob owns a unique reference to the StringRepresentation. +This is because StringBlob, implements an interface which should work across threads. */ class StringBlob : public BlobBase { @@ -46,37 +45,39 @@ public: virtual SLANG_NO_THROW void* SLANG_MCALL castAs(const SlangUUID& guid) SLANG_OVERRIDE; // ISlangBlob - SLANG_NO_THROW void const* SLANG_MCALL getBufferPointer() SLANG_OVERRIDE { return m_string.getBuffer(); } - SLANG_NO_THROW size_t SLANG_MCALL getBufferSize() SLANG_OVERRIDE { return m_string.getLength(); } + SLANG_NO_THROW void const* SLANG_MCALL getBufferPointer() SLANG_OVERRIDE { return m_chars; } + SLANG_NO_THROW size_t SLANG_MCALL getBufferSize() SLANG_OVERRIDE { return m_charsCount; } - static ComPtr create(const String& in) { return ComPtr(new StringBlob(in)); } + /// Since in is not being moved will *always* create a new representation, unless the in is empty + static ComPtr create(const String& in); + /// Create from a slice + static ComPtr create(const UnownedStringSlice& slice); /// Moves from in into the created blob. /// NOTE! That will only use the representation from in, if it is *unique* /// otherwise it will make a new copy. - /// This is so that StringBlob won't hold a reference count via a string held externally. - /// In contrast StringBlob::create *may* share the representation. static ComPtr moveCreate(String& in); static ComPtr moveCreate(String&& in); -protected: - /// A type that is only used to differentiate a constructor. Can construct with - /// MoveUnique{} - struct MoveUnique {}; - - explicit StringBlob(String const& string) - : m_string(string) - {} + /// Dtor + ~StringBlob(); - StringBlob(MoveUnique, String& string); - StringBlob() {} +protected: + /// Use when rep *must* be unique (ie nullptr, and has a ref count of 1, that can be `taken` by the blob + void _uniqueInit(StringRepresentation* uniqueRep); + /// Init with a rep when can't be owned. + void _init(StringRepresentation* rep); + /// Init with a representation that has been moved. + void _moveInit(StringRepresentation* rep); - /// Get the contained string - SLANG_FORCE_INLINE const String& getString() const { return m_string; } + /// Checks that m_rep is either nullptr or has a ref count of 1 (ie it is owned by the blob) + SLANG_FORCE_INLINE void _checkRep() const { SLANG_ASSERT(m_rep == nullptr || m_rep->isUniquelyReferenced()); } void* getObject(const Guid& guid); - String m_string; + char* m_chars = nullptr; ///< Pointer to the contained data. + size_t m_charsCount = 0; ///< The amount of chars *not* including terminating 0 + StringRepresentation* m_rep = nullptr; ///< Holds actual bytes. Can be nullptr if it's an empty string. }; class ListBlob : public BlobBase diff --git a/source/core/slang-string.h b/source/core/slang-string.h index 429a60c46..b4583d0b2 100644 --- a/source/core/slang-string.h +++ b/source/core/slang-string.h @@ -474,8 +474,12 @@ namespace Slang /// Append data written to buffer output via 'prepareForAppend' directly written 'inplace' void appendInPlace(const char* chars, Index count); + /// Get the internal string represenation SLANG_FORCE_INLINE StringRepresentation* getStringRepresentation() const { return m_buffer; } + /// Detach the representation (will leave string as empty). Rep ref count will remain unchanged. + SLANG_FORCE_INLINE StringRepresentation* detachStringRepresentation() { return m_buffer.detach(); } + const char* begin() const { return getData(); -- cgit v1.2.3