Bug 1663382 - Optimize Deserializer<ProfilerStringView> to reference in-buffer data when possible - r=canaltinova

Because string contents could be split in two separate chunks, the default ProfilerStringView deserializer needed to concatenate it together in an off-chunk buffer.
But now thanks to ProfileBufferEntryReader::ReadSpans, it is possible to know if the contents are in a single memory area inside one chunk (which should be the vast majority of cases), in which case the ProfilerStringView can just reference it using its internal std::string_view, which saves managing a separate buffer and copying data into it.
However this can only be done safely when the span is correctly aligned for the character type, which may not be the case for char16_t strings that must be even-aligned.

Differential Revision: https://phabricator.services.mozilla.com/D124430

mozglue/baseprofiler/public/BaseProfilerMarkersDetail.h

@@ -422,25 +422,7 @@ template <typename CHAR>
 struct ProfileBufferEntryReader::Deserializer<ProfilerStringView<CHAR>> {
   static void ReadInto(ProfileBufferEntryReader& aER,
                        ProfilerStringView<CHAR>& aString) {
-    const Length lengthAndIsLiteral = aER.ReadULEB128<Length>();
-    const Length stringLength = lengthAndIsLiteral >> 1;
-    if ((lengthAndIsLiteral & 1u) == 0u) {
-      // LSB==0 -> Literal string, read the string pointer.
-      aString.mStringView = std::basic_string_view<CHAR>(
-          aER.ReadObject<const CHAR*>(), stringLength);
-      aString.mOwnership = ProfilerStringView<CHAR>::Ownership::Literal;
-      return;
-    }
-    // LSB==1 -> Not a literal string, allocate a buffer to store the string
-    // (plus terminal, for safety), and give it to the ProfilerStringView; Note
-    // that this is a secret use of ProfilerStringView, which is intended to
-    // only be used between deserialization and JSON streaming.
-    CHAR* buffer = new CHAR[stringLength + 1];
-    aER.ReadBytes(buffer, stringLength * sizeof(CHAR));
-    buffer[stringLength] = CHAR(0);
-    aString.mStringView = std::basic_string_view<CHAR>(buffer, stringLength);
-    aString.mOwnership =
-        ProfilerStringView<CHAR>::Ownership::OwnedThroughStringView;
+    aString = Read(aER);
   }
 
   static ProfilerStringView<CHAR> Read(ProfileBufferEntryReader& aER) {
@@ -452,16 +434,32 @@ struct ProfileBufferEntryReader::Deserializer<ProfilerStringView<CHAR>> {
           aER.ReadObject<const CHAR*>(), stringLength,
           ProfilerStringView<CHAR>::Ownership::Literal);
     }
-    // LSB==1 -> Not a literal string, allocate a buffer to store the string
-    // (plus terminal, for safety), and give it to the ProfilerStringView; Note
-    // that this is a secret use of ProfilerStringView, which is intended to
-    // only be used between deserialization and JSON streaming.
-    CHAR* buffer = new CHAR[stringLength + 1];
-    aER.ReadBytes(buffer, stringLength * sizeof(CHAR));
-    buffer[stringLength] = CHAR(0);
-    return ProfilerStringView<CHAR>(
-        buffer, stringLength,
-        ProfilerStringView<CHAR>::Ownership::OwnedThroughStringView);
+    // LSB==1 -> Not a literal string.
+    ProfileBufferEntryReader::DoubleSpanOfConstBytes spans =
+        aER.ReadSpans(stringLength * sizeof(CHAR));
+    if (MOZ_LIKELY(spans.IsSingleSpan()) &&
+        reinterpret_cast<uintptr_t>(spans.mFirstOrOnly.Elements()) %
+                alignof(CHAR) ==
+            0u) {
+      // Only a single span, correctly aligned for the CHAR type, we can just
+      // refer to it directly, assuming that this ProfilerStringView will not
+      // outlive the chunk.
+      return ProfilerStringView<CHAR>(
+          reinterpret_cast<const CHAR*>(spans.mFirstOrOnly.Elements()),
+          stringLength, ProfilerStringView<CHAR>::Ownership::Reference);
+    } else {
+      // Two spans, we need to concatenate them; or one span, but misaligned.
+      // Allocate a buffer to store the string (plus terminal, for safety), and
+      // give it to the ProfilerStringView; Note that this is a secret use of
+      // ProfilerStringView, which is intended to only be used between
+      // deserialization and JSON streaming.
+      CHAR* buffer = new CHAR[stringLength + 1];
+      spans.CopyBytesTo(buffer);
+      buffer[stringLength] = CHAR(0);
+      return ProfilerStringView<CHAR>(
+          buffer, stringLength,
+          ProfilerStringView<CHAR>::Ownership::OwnedThroughStringView);
+    }
   }
 };
 

mozglue/tests/TestBaseProfiler.cpp

@@ -3773,40 +3773,66 @@ void TestProfilerStringView() {
     MOZ_RELEASE_ASSERT(!outerBSV.IsReference());
   }
 
+  MOZ_RELEASE_ASSERT(cb.GetState().mRangeStart == 1u);
+
   cb.Clear();
 
   // Non-literal string, content is serialized.
-  std::basic_string<CHAR> hiString(hi);
-  MOZ_RELEASE_ASSERT(cb.PutObject(BSV(hiString)));
+
+  // We'll try to write 4 strings, such that the 4th one will cross into the
+  // next chunk.
+  unsigned guessedChunkBytes = unsigned(cb.GetState().mRangeStart) - 1u;
+  static constexpr unsigned stringCount = 4u;
+  const unsigned stringSize =
+      guessedChunkBytes / stringCount / sizeof(CHAR) + 3u;
+
+  std::basic_string<CHAR> longString;
+  longString.reserve(stringSize);
+  for (unsigned i = 0; i < stringSize; ++i) {
+    longString += CHAR('0' + i);
+  }
+
+  for (unsigned i = 0; i < stringCount; ++i) {
+    MOZ_RELEASE_ASSERT(cb.PutObject(BSV(longString)));
+  }
+
   {
     unsigned read = 0;
     ProfilerStringView<CHAR> outerBSV;
     cb.ReadEach([&](ProfileBufferEntryReader& aER) {
       ++read;
-      auto bsv = aER.ReadObject<ProfilerStringView<CHAR>>();
-      MOZ_RELEASE_ASSERT(bsv.Data());
-      MOZ_RELEASE_ASSERT(bsv.Data() != hiString.data());
-      MOZ_RELEASE_ASSERT(bsv.Data()[0] == CHAR('h'));
-      MOZ_RELEASE_ASSERT(bsv.Data()[1] == CHAR('i'));
-      MOZ_RELEASE_ASSERT(bsv.Data()[2] == CHAR('\0'));
-      MOZ_RELEASE_ASSERT(bsv.Length() == 2);
-      // Special ownership case, neither a literal nor a reference!
-      MOZ_RELEASE_ASSERT(!bsv.IsLiteral());
-      MOZ_RELEASE_ASSERT(!bsv.IsReference());
-      // Test move of ownership.
-      outerBSV = std::move(bsv);
-      // NOLINTNEXTLINE(bugprone-use-after-move, clang-analyzer-cplusplus.Move)
-      MOZ_RELEASE_ASSERT(bsv.Length() == 0);
+      {
+        auto bsv = aER.ReadObject<ProfilerStringView<CHAR>>();
+        MOZ_RELEASE_ASSERT(bsv.Length() == stringSize);
+        MOZ_RELEASE_ASSERT(bsv.Data());
+        for (unsigned i = 0; i < stringSize; ++i) {
+          MOZ_RELEASE_ASSERT(bsv.Data()[i] == CHAR('0' + i));
+          longString += '0' + i;
+        }
+        MOZ_RELEASE_ASSERT(!bsv.IsLiteral());
+        // The first 3 should be references (because they fit in one chunk, so
+        // they can be referenced directly), which the 4th one have to be copied
+        // out of two chunks and stitched back together.
+        MOZ_RELEASE_ASSERT(bsv.IsReference() == (read != 4));
+
+        // Test move of ownership.
+        outerBSV = std::move(bsv);
+        // After a move, references stay complete, while a non-reference had a
+        // buffer that has been moved out.
+        // NOLINTNEXTLINE(bugprone-use-after-move,clang-analyzer-cplusplus.Move)
+        MOZ_RELEASE_ASSERT(bsv.Length() == ((read != 4) ? stringSize : 0));
+      }
+
+      MOZ_RELEASE_ASSERT(outerBSV.Length() == stringSize);
+      MOZ_RELEASE_ASSERT(outerBSV.Data());
+      for (unsigned i = 0; i < stringSize; ++i) {
+        MOZ_RELEASE_ASSERT(outerBSV.Data()[i] == CHAR('0' + i));
+        longString += '0' + i;
+      }
+      MOZ_RELEASE_ASSERT(!outerBSV.IsLiteral());
+      MOZ_RELEASE_ASSERT(outerBSV.IsReference() == (read != 4));
     });
-    MOZ_RELEASE_ASSERT(read == 1);
-    MOZ_RELEASE_ASSERT(outerBSV.Data());
-    MOZ_RELEASE_ASSERT(outerBSV.Data() != hiString.data());
-    MOZ_RELEASE_ASSERT(outerBSV.Data()[0] == CHAR('h'));
-    MOZ_RELEASE_ASSERT(outerBSV.Data()[1] == CHAR('i'));
-    MOZ_RELEASE_ASSERT(outerBSV.Data()[2] == CHAR('\0'));
-    MOZ_RELEASE_ASSERT(outerBSV.Length() == 2);
-    MOZ_RELEASE_ASSERT(!outerBSV.IsLiteral());
-    MOZ_RELEASE_ASSERT(!outerBSV.IsReference());
+    MOZ_RELEASE_ASSERT(read == 4);
   }
 
   if constexpr (std::is_same_v<CHAR, char>) {