From 09671823e4f0ab8647d456c92a17d38fa4e66099 Mon Sep 17 00:00:00 2001 From: Wojtek Figat Date: Thu, 29 Apr 2021 23:24:06 +0200 Subject: [PATCH] Optimize String usage with StringView for basic file paths operations --- .../ContentImporters/ImportModelFile.cpp | 3 +- Source/Engine/Core/Types/Guid.cpp | 8 +-- Source/Engine/Core/Types/String.cpp | 15 +++- Source/Engine/Core/Types/String.h | 20 +++++- Source/Engine/Core/Types/StringView.cpp | 16 ++--- Source/Engine/Core/Types/StringView.h | 8 +-- Source/Engine/Platform/Base/PlatformBase.cpp | 2 +- .../Engine/Platform/Base/StringUtilsBase.cpp | 72 ++++++------------- Source/Engine/Platform/StringUtils.h | 8 +-- .../Scripting/ManagedCLR/MAssembly.Mono.cpp | 6 +- Source/Engine/Terrain/TerrainPatch.cpp | 2 +- .../Tools/ModelTool/ModelTool.OpenFBX.cpp | 6 +- Source/Engine/Tools/ModelTool/ModelTool.cpp | 4 +- 13 files changed, 84 insertions(+), 86 deletions(-) diff --git a/Source/Engine/ContentImporters/ImportModelFile.cpp b/Source/Engine/ContentImporters/ImportModelFile.cpp index 073f3edbd..32fd38e4d 100644 --- a/Source/Engine/ContentImporters/ImportModelFile.cpp +++ b/Source/Engine/ContentImporters/ImportModelFile.cpp @@ -115,7 +115,8 @@ CreateAssetResult ImportModelFile::Import(CreateAssetContext& context) // Import model file ModelData modelData; String errorMsg; - if (ModelTool::ImportModel(context.InputPath, modelData, options, errorMsg, StringUtils::GetDirectoryName(context.TargetAssetPath) / StringUtils::GetFileNameWithoutExtension(context.InputPath))) + String autoImportOutput = String(StringUtils::GetDirectoryName(context.TargetAssetPath)) / StringUtils::GetFileNameWithoutExtension(context.InputPath); + if (ModelTool::ImportModel(context.InputPath, modelData, options, errorMsg, autoImportOutput)) { LOG(Error, "Cannot import model file. {0}", errorMsg); return CreateAssetResult::Error; diff --git a/Source/Engine/Core/Types/Guid.cpp b/Source/Engine/Core/Types/Guid.cpp index 323949646..c8aaf8aa1 100644 --- a/Source/Engine/Core/Types/Guid.cpp +++ b/Source/Engine/Core/Types/Guid.cpp @@ -101,8 +101,8 @@ FORCE_INLINE bool GuidParse(const StringViewType& text, Guid& value) // FormatType::D case 36: { - StringType b = text.Substring(9, 4) + text.Substring(14, 4); - StringType c = text.Substring(19, 4) + text.Substring(24, 4); + StringType b = StringType(text.Substring(9, 4)) + text.Substring(14, 4); + StringType c = StringType(text.Substring(19, 4)) + text.Substring(24, 4); return StringUtils::ParseHex(*text + 0, 8, &value.A) || StringUtils::ParseHex(*b, &value.B) || @@ -113,8 +113,8 @@ FORCE_INLINE bool GuidParse(const StringViewType& text, Guid& value) // FormatType::P case 38: { - StringType b = text.Substring(10, 4) + text.Substring(15, 4); - StringType c = text.Substring(20, 4) + text.Substring(25, 4); + StringType b = StringType(text.Substring(10, 4)) + text.Substring(15, 4); + StringType c = StringType(text.Substring(20, 4)) + text.Substring(25, 4); return text[0] != text[text.Length() - 1] || StringUtils::ParseHex(*text + 1, 8, &value.A) || diff --git a/Source/Engine/Core/Types/String.cpp b/Source/Engine/Core/Types/String.cpp index 040bb9ab2..7919d513d 100644 --- a/Source/Engine/Core/Types/String.cpp +++ b/Source/Engine/Core/Types/String.cpp @@ -13,7 +13,18 @@ String::String(const StringAnsi& str) String::String(const StringView& str) { - Set(str.Get(), str.Length()); + _length = str.Length(); + if (_length != 0) + { + ASSERT(_length > 0); + _data = (Char*)Platform::Allocate((_length + 1) * sizeof(Char), 16); + _data[_length] = 0; + Platform::MemoryCopy(_data, str.Get(), _length * sizeof(Char)); + } + else + { + _data = nullptr; + } } String::String(const StringAnsiView& str) @@ -38,7 +49,6 @@ void String::Set(const Char* chars, int32 length) } _length = length; } - Platform::MemoryCopy(_data, chars, length * sizeof(Char)); } @@ -58,7 +68,6 @@ void String::Set(const char* chars, int32 length) } _length = length; } - if (chars) StringUtils::ConvertANSI2UTF16(chars, _data, length); } diff --git a/Source/Engine/Core/Types/String.h b/Source/Engine/Core/Types/String.h index 75d484303..d8684bf67 100644 --- a/Source/Engine/Core/Types/String.h +++ b/Source/Engine/Core/Types/String.h @@ -539,6 +539,7 @@ public: /// /// The reference to the string. String(const String& str) + : String() { Set(str.Get(), str.Length()); } @@ -576,6 +577,7 @@ public: /// The ANSI string. /// The ANSI string length. explicit String(const char* str, int32 length) + : String() { Set(str, length); } @@ -585,8 +587,9 @@ public: /// /// The UTF-16 string. String(const Char* str) - : String(str, StringUtils::Length(str)) + : String() { + Set(str, StringUtils::Length(str)); } /// @@ -595,6 +598,7 @@ public: /// The UTF-16 string. /// The UTF-16 string length. String(const Char* str, int32 length) + : String() { Set(str, length); } @@ -603,7 +607,7 @@ public: /// Initializes a new instance of the class. /// /// The other string. - explicit String(const StringView& str); + String(const StringView& str); /// /// Initializes a new instance of the class. @@ -1110,7 +1114,17 @@ public: /// The combined path. FORCE_INLINE String operator/(const String& str) const { - return operator/(*str); + return String(*this) /= str; + } + + /// + /// Concatenates this path with given path ensuring the '/' character is used between them. + /// + /// The string to be concatenated onto the end of this. + /// The combined path. + FORCE_INLINE String operator/(const StringView& str) const + { + return String(*this) /= str; } public: diff --git a/Source/Engine/Core/Types/StringView.cpp b/Source/Engine/Core/Types/StringView.cpp index e29afe4e8..8b30cc755 100644 --- a/Source/Engine/Core/Types/StringView.cpp +++ b/Source/Engine/Core/Types/StringView.cpp @@ -27,28 +27,28 @@ bool StringView::operator!=(const String& other) const return StringUtils::Compare(this->GetText(), *other) != 0; } -String StringView::Left(int32 count) const +StringView StringView::Left(int32 count) const { const int32 countClamped = count < 0 ? 0 : count < Length() ? count : Length(); - return String(**this, countClamped); + return StringView(**this, countClamped); } -String StringView::Right(int32 count) const +StringView StringView::Right(int32 count) const { const int32 countClamped = count < 0 ? 0 : count < Length() ? count : Length(); - return String(**this + Length() - countClamped); + return StringView(**this + Length() - countClamped); } -String StringView::Substring(int32 startIndex) const +StringView StringView::Substring(int32 startIndex) const { ASSERT(startIndex >= 0 && startIndex < Length()); - return String(Get() + startIndex, Length() - startIndex); + return StringView(Get() + startIndex, Length() - startIndex); } -String StringView::Substring(int32 startIndex, int32 count) const +StringView StringView::Substring(int32 startIndex, int32 count) const { ASSERT(startIndex >= 0 && startIndex + count <= Length() && count >= 0); - return String(Get() + startIndex, count); + return StringView(Get() + startIndex, count); } String StringView::ToString() const diff --git a/Source/Engine/Core/Types/StringView.h b/Source/Engine/Core/Types/StringView.h index fe245e8f2..e267b760e 100644 --- a/Source/Engine/Core/Types/StringView.h +++ b/Source/Engine/Core/Types/StringView.h @@ -320,21 +320,21 @@ public: /// /// The characters count. /// The substring. - String Left(int32 count) const; + StringView Left(int32 count) const; /// /// Gets the string of characters from the right (end of the string). /// /// The characters count. /// The substring. - String Right(int32 count) const; + StringView Right(int32 count) const; /// /// Retrieves substring created from characters starting from startIndex to the String end. /// /// The index of the first character to subtract. /// The substring created from String data. - String Substring(int32 startIndex) const; + StringView Substring(int32 startIndex) const; /// /// Retrieves substring created from characters starting from start index. @@ -342,7 +342,7 @@ public: /// The index of the first character to subtract. /// The amount of characters to retrieve. /// The substring created from String data. - String Substring(int32 startIndex, int32 count) const; + StringView Substring(int32 startIndex, int32 count) const; public: diff --git a/Source/Engine/Platform/Base/PlatformBase.cpp b/Source/Engine/Platform/Base/PlatformBase.cpp index b47f3c076..751f63dab 100644 --- a/Source/Engine/Platform/Base/PlatformBase.cpp +++ b/Source/Engine/Platform/Base/PlatformBase.cpp @@ -261,7 +261,7 @@ void PlatformBase::Fatal(const Char* msg, void* context) } // Create separate folder with crash info - const String crashDataFolder = StringUtils::GetDirectoryName(Log::Logger::LogFilePath) / TEXT("Crash_") + StringUtils::GetFileNameWithoutExtension(Log::Logger::LogFilePath).Substring(4); + const String crashDataFolder = String(StringUtils::GetDirectoryName(Log::Logger::LogFilePath)) / TEXT("Crash_") + StringUtils::GetFileNameWithoutExtension(Log::Logger::LogFilePath).Substring(4); FileSystem::CreateDirectory(crashDataFolder); // Capture the platform-dependant crash info (eg. memory dump) diff --git a/Source/Engine/Platform/Base/StringUtilsBase.cpp b/Source/Engine/Platform/Base/StringUtilsBase.cpp index e8fe3ebc3..bf90282e8 100644 --- a/Source/Engine/Platform/Base/StringUtilsBase.cpp +++ b/Source/Engine/Platform/Base/StringUtilsBase.cpp @@ -289,15 +289,15 @@ void RemoveLongPathPrefix(const String& path, String& result) result.Remove(2, 6); } -String StringUtils::GetDirectoryName(const StringView& path) +StringView StringUtils::GetDirectoryName(const StringView& path) { const int32 lastFrontSlash = path.FindLast('\\'); const int32 lastBackSlash = path.FindLast('/'); const int32 splitIndex = Math::Max(lastBackSlash, lastFrontSlash); - return splitIndex != INVALID_INDEX ? path.Left(splitIndex) : String::Empty; + return splitIndex != INVALID_INDEX ? path.Left(splitIndex) : StringView::Empty; } -String StringUtils::GetFileName(const StringView& path) +StringView StringUtils::GetFileName(const StringView& path) { Char chr; const int32 length = path.Length(); @@ -306,31 +306,27 @@ String StringUtils::GetFileName(const StringView& path) { num--; if (num < 0) - return String(path); + return path; chr = path[num]; } while (chr != DirectorySeparatorChar && chr != AltDirectorySeparatorChar && chr != VolumeSeparatorChar); return path.Substring(num + 1, length - num - 1); } -String StringUtils::GetFileNameWithoutExtension(const StringView& path) +StringView StringUtils::GetFileNameWithoutExtension(const StringView& path) { - String filename = GetFileName(path); + StringView filename = GetFileName(path); const int32 num = filename.FindLast('.'); if (num != -1) - { return filename.Substring(0, num); - } return filename; } -String StringUtils::GetPathWithoutExtension(const StringView& path) +StringView StringUtils::GetPathWithoutExtension(const StringView& path) { const int32 num = path.FindLast('.'); if (num != -1) - { return path.Substring(0, num); - } - return String(path); + return path; } void StringUtils::PathRemoveRelativeParts(String& path) @@ -369,7 +365,7 @@ void StringUtils::PathRemoveRelativeParts(String& path) } } - bool isRooted = path.StartsWith(TEXT('/')); + const bool isRooted = path.StartsWith(TEXT('/')); path.Clear(); for (auto& e : stack) path /= e; @@ -377,7 +373,7 @@ void StringUtils::PathRemoveRelativeParts(String& path) path.Insert(0, TEXT("/")); } -const char digit_pairs[201] = { +const char DigitPairs[201] = { "00010203040506070809" "10111213141516171819" "20212223242526272829" @@ -414,21 +410,17 @@ String StringUtils::ToString(int32 value) { char buf[STRING_UTILS_ITOSTR_BUFFER_SIZE]; char* it = &buf[STRING_UTILS_ITOSTR_BUFFER_SIZE - 2]; - int32 div = value / 100; - if (value >= 0) { while (div) { - Platform::MemoryCopy(it, &digit_pairs[2 * (value - div * 100)], 2); + Platform::MemoryCopy(it, &DigitPairs[2 * (value - div * 100)], 2); value = div; it -= 2; div = value / 100; } - - Platform::MemoryCopy(it, &digit_pairs[2 * value], 2); - + Platform::MemoryCopy(it, &DigitPairs[2 * value], 2); if (value < 10) it++; } @@ -436,20 +428,16 @@ String StringUtils::ToString(int32 value) { while (div) { - Platform::MemoryCopy(it, &digit_pairs[-2 * (value - div * 100)], 2); + Platform::MemoryCopy(it, &DigitPairs[-2 * (value - div * 100)], 2); value = div; it -= 2; div = value / 100; } - - Platform::MemoryCopy(it, &digit_pairs[-2 * value], 2); - + Platform::MemoryCopy(it, &DigitPairs[-2 * value], 2); if (value <= -10) it--; - *it = '-'; } - return String(it, (int32)(&buf[STRING_UTILS_ITOSTR_BUFFER_SIZE] - it)); } @@ -457,21 +445,17 @@ String StringUtils::ToString(int64 value) { char buf[STRING_UTILS_ITOSTR_BUFFER_SIZE]; char* it = &buf[STRING_UTILS_ITOSTR_BUFFER_SIZE - 2]; - int64 div = value / 100; - if (value >= 0) { while (div) { - Platform::MemoryCopy(it, &digit_pairs[2 * (value - div * 100)], 2); + Platform::MemoryCopy(it, &DigitPairs[2 * (value - div * 100)], 2); value = div; it -= 2; div = value / 100; } - - Platform::MemoryCopy(it, &digit_pairs[2 * value], 2); - + Platform::MemoryCopy(it, &DigitPairs[2 * value], 2); if (value < 10) it++; } @@ -479,20 +463,16 @@ String StringUtils::ToString(int64 value) { while (div) { - Platform::MemoryCopy(it, &digit_pairs[-2 * (value - div * 100)], 2); + Platform::MemoryCopy(it, &DigitPairs[-2 * (value - div * 100)], 2); value = div; it -= 2; div = value / 100; } - - Platform::MemoryCopy(it, &digit_pairs[-2 * value], 2); - + Platform::MemoryCopy(it, &DigitPairs[-2 * value], 2); if (value <= -10) it--; - *it = '-'; } - return String(it, (int32)(&buf[STRING_UTILS_ITOSTR_BUFFER_SIZE] - it)); } @@ -500,21 +480,17 @@ String StringUtils::ToString(uint32 value) { char buf[STRING_UTILS_ITOSTR_BUFFER_SIZE]; char* it = &buf[STRING_UTILS_ITOSTR_BUFFER_SIZE - 2]; - int32 div = value / 100; while (div) { - Platform::MemoryCopy(it, &digit_pairs[2 * (value - div * 100)], 2); + Platform::MemoryCopy(it, &DigitPairs[2 * (value - div * 100)], 2); value = div; it -= 2; div = value / 100; } - - Platform::MemoryCopy(it, &digit_pairs[2 * value], 2); - + Platform::MemoryCopy(it, &DigitPairs[2 * value], 2); if (value < 10) it++; - return String((char*)it, (int32)((char*)&buf[STRING_UTILS_ITOSTR_BUFFER_SIZE] - (char*)it)); } @@ -522,21 +498,17 @@ String StringUtils::ToString(uint64 value) { char buf[STRING_UTILS_ITOSTR_BUFFER_SIZE]; char* it = &buf[STRING_UTILS_ITOSTR_BUFFER_SIZE - 2]; - int64 div = value / 100; while (div) { - Platform::MemoryCopy(it, &digit_pairs[2 * (value - div * 100)], 2); + Platform::MemoryCopy(it, &DigitPairs[2 * (value - div * 100)], 2); value = div; it -= 2; div = value / 100; } - - Platform::MemoryCopy(it, &digit_pairs[2 * value], 2); - + Platform::MemoryCopy(it, &DigitPairs[2 * value], 2); if (value < 10) it++; - return String((char*)it, (int32)((char*)&buf[STRING_UTILS_ITOSTR_BUFFER_SIZE] - (char*)it)); } diff --git a/Source/Engine/Platform/StringUtils.h b/Source/Engine/Platform/StringUtils.h index 9d812bc47..101049b8e 100644 --- a/Source/Engine/Platform/StringUtils.h +++ b/Source/Engine/Platform/StringUtils.h @@ -207,19 +207,19 @@ public: // Returns the directory name of the specified path string // @param path The path string from which to obtain the directory name // @returns Directory name - static String GetDirectoryName(const StringView& path); + static StringView GetDirectoryName(const StringView& path); // Returns the file name and extension of the specified path string // @param path The path string from which to obtain the file name and extension // @returns File name with extension - static String GetFileName(const StringView& path); + static StringView GetFileName(const StringView& path); // Returns the file name without extension of the specified path string // @param path The path string from which to obtain the file name // @returns File name without extension - static String GetFileNameWithoutExtension(const StringView& path); + static StringView GetFileNameWithoutExtension(const StringView& path); - static String GetPathWithoutExtension(const StringView& path); + static StringView GetPathWithoutExtension(const StringView& path); static void PathRemoveRelativeParts(String& path); diff --git a/Source/Engine/Scripting/ManagedCLR/MAssembly.Mono.cpp b/Source/Engine/Scripting/ManagedCLR/MAssembly.Mono.cpp index f24429da4..32ae90870 100644 --- a/Source/Engine/Scripting/ManagedCLR/MAssembly.Mono.cpp +++ b/Source/Engine/Scripting/ManagedCLR/MAssembly.Mono.cpp @@ -316,7 +316,7 @@ bool MAssembly::LoadWithImage(const String& assemblyPath) #if MONO_DEBUG_ENABLE // Try to load debug symbols (use portable PDB format) - const auto pdbPath = StringUtils::GetPathWithoutExtension(assemblyPath) + TEXT(".pdb"); + const auto pdbPath = String(StringUtils::GetPathWithoutExtension(assemblyPath)) + TEXT(".pdb"); if (FileSystem::FileExists(pdbPath)) { // Load .pdb file @@ -334,10 +334,10 @@ bool MAssembly::LoadWithImage(const String& assemblyPath) if (assemblyPath.EndsWith(TEXT("FlaxEngine.CSharp.dll"))) { static Array NewtonsoftJsonDebugData; - File::ReadAllBytes(StringUtils::GetDirectoryName(assemblyPath) / TEXT("Newtonsoft.Json.pdb"), NewtonsoftJsonDebugData); + File::ReadAllBytes(String(StringUtils::GetDirectoryName(assemblyPath)) / TEXT("Newtonsoft.Json.pdb"), NewtonsoftJsonDebugData); if (NewtonsoftJsonDebugData.HasItems()) { - StringAnsi tmp(StringUtils::GetDirectoryName(assemblyPath) / TEXT("Newtonsoft.Json.dll")); + StringAnsi tmp(String(StringUtils::GetDirectoryName(assemblyPath)) / TEXT("Newtonsoft.Json.dll")); MonoAssembly* a = mono_assembly_open(tmp.Get(), &status); if (a) { diff --git a/Source/Engine/Terrain/TerrainPatch.cpp b/Source/Engine/Terrain/TerrainPatch.cpp index fb38b77b8..f96fec22d 100644 --- a/Source/Engine/Terrain/TerrainPatch.cpp +++ b/Source/Engine/Terrain/TerrainPatch.cpp @@ -1684,7 +1684,7 @@ bool TerrainPatch::ModifySplatMap(int32 index, const Color32* samples, const Int else { // Prepare asset path for the non-virtual asset - const String cacheDir = StringUtils::GetDirectoryName(Heightmap->GetPath()) / _terrain->GetID().ToString(Guid::FormatType::N); + const String cacheDir = String(StringUtils::GetDirectoryName(Heightmap->GetPath())) / _terrain->GetID().ToString(Guid::FormatType::N); const String splatMapPath = cacheDir + String::Format(TEXT("_{0:2}_{1:2}_Splatmap{3}.{2}"), _x, _z, ASSET_FILES_EXTENSION, index); // Import data to the asset file diff --git a/Source/Engine/Tools/ModelTool/ModelTool.OpenFBX.cpp b/Source/Engine/Tools/ModelTool/ModelTool.OpenFBX.cpp index 2e77267b5..db8660b77 100644 --- a/Source/Engine/Tools/ModelTool/ModelTool.OpenFBX.cpp +++ b/Source/Engine/Tools/ModelTool/ModelTool.OpenFBX.cpp @@ -1085,10 +1085,12 @@ bool ModelTool::ImportDataOpenFBX(const char* path, ImportedModelData& data, con aFilename.toString(filenameData); if (outputPath.IsEmpty()) { - outputPath = StringUtils::GetDirectoryName(String(path)) / TEXT("textures"); + String pathStr(path); + outputPath = String(StringUtils::GetDirectoryName(pathStr)) / TEXT("textures"); FileSystem::CreateDirectory(outputPath); } - String embeddedPath = outputPath / StringUtils::GetFileName(String(filenameData)); + const String filenameStr(filenameData); + String embeddedPath = outputPath / StringUtils::GetFileName(filenameStr); if (FileSystem::FileExists(embeddedPath)) continue; LOG(Info, "Extracing embedded resource to {0}", embeddedPath); diff --git a/Source/Engine/Tools/ModelTool/ModelTool.cpp b/Source/Engine/Tools/ModelTool/ModelTool.cpp index 4fb9496d4..06c5bac13 100644 --- a/Source/Engine/Tools/ModelTool/ModelTool.cpp +++ b/Source/Engine/Tools/ModelTool/ModelTool.cpp @@ -487,7 +487,7 @@ bool ModelTool::ImportModel(const String& path, ModelData& meshData, Options opt // Auto-import textures if (autoImportOutput.IsEmpty() || (data.Types & ImportDataTypes::Textures) == 0 || texture.FilePath.IsEmpty()) continue; - auto filename = StringUtils::GetFileNameWithoutExtension(texture.FilePath); + String filename = StringUtils::GetFileNameWithoutExtension(texture.FilePath); for (int32 j = filename.Length() - 1; j >= 0; j--) { if (EditorUtilities::IsInvalidPathChar(filename[j])) @@ -498,7 +498,7 @@ bool ModelTool::ImportModel(const String& path, ModelData& meshData, Options opt int32 counter = 1; do { - filename = StringUtils::GetFileNameWithoutExtension(texture.FilePath) + TEXT(" ") + StringUtils::ToString(counter); + filename = String(StringUtils::GetFileNameWithoutExtension(texture.FilePath)) + TEXT(" ") + StringUtils::ToString(counter); counter++; } while (importedFileNames.Contains(filename)); }