[scudo] Optimize scudo test string allocation

[ddcc]

Summary

When the underlying vector becomes full, it resizes, remaps, and then copies over the old data. To avoid thes excess allocations, allow reservation from the backing vector.

[vitalybuka]

compiler-rt/lib/scudo/standalone/string_utils.h

22

I don't like inconsistency with std:: or even Vector<> here
the Size argument is expected to be the new size of the container, not reserved size.

It the optimization is important, I would prefer something like:
explicit ScopedString() = default;
uptr length() { return Max(String.size(), 1) - 1; }
const char *data() { return String.size() ? String.data() : ""; }

[ddcc]

compiler-rt/lib/scudo/standalone/string_utils.h

22

How about just exposing the reserve method of the underlying container then? Where the reserved size is always incremented by one to account for the terminator.

[vitalybuka]

compiler-rt/lib/scudo/standalone/string_utils.h

22

lgtm

[ddcc]

Call new reserve() method

[vitalybuka]

Please update descriptions

[vitalybuka]

compiler-rt/lib/scudo/standalone/string_utils.h

31

compiler-rt/lib/scudo/standalone/string_utils.h

 
 #include <stdarg.h>
 
 namespace scudo {
 
 class ScopedString {
 public:
   explicit ScopedString() { String.push_back('\0'); }
   uptr length() { return String.size() - 1; }
   const char *data() { return String.data(); }
   void clear() {
     String.clear();
     String.push_back('\0');
   }
   void append(const char *Format, va_list Args);
   void append(const char *Format, ...) FORMAT(2, 3);
   void output() const { outputRaw(String.data()); }
+  void reserve(size_t Size) { String.reserve(Size + 1); }
 
 private:
   Vector<char> String;
 };
 
 int formatString(char *Buffer, uptr BufferLength, const char *Format, ...)
     FORMAT(3, 4);
 void Printf(const char *Format, ...) FORMAT(1, 2);
 
 } // namespace scudo
 
 #endif // SCUDO_STRING_UTILS_H_

compiler-rt/lib/scudo/standalone/tests/strings_test.cpp

TEST(ScudoStringsTest, Clear) {
   scudo::ScopedString Str;
   Str.append("123");
   Str.clear();
   EXPECT_EQ(0ul, Str.length());
   EXPECT_EQ('\0', *Str.data());
 }
 
 TEST(ScudoStringsTest, ClearLarge) {
+  constexpr char appendString[] = "123";
   scudo::ScopedString Str;
+  Str.reserve(sizeof(appendString) * 10000);
   for (int i = 0; i < 10000; ++i)
-    Str.append("123");
+    Str.append(appendString);
   Str.clear();
   EXPECT_EQ(0ul, Str.length());
   EXPECT_EQ('\0', *Str.data());
 }
 
 TEST(ScudoStringsTest, Precision) {
   scudo::ScopedString Str;
   Str.append("%.*s", 3, "12345");
for (scudo::uptr I = 0; I < Size; I++)
     Str.append("A");
 }
 
 TEST(ScudoStringTest, PotentialOverflows) {
   // Use a ScopedString that spans a page, and attempt to write past the end
   // of it with variations of append. The expectation is for nothing to crash.
   const scudo::uptr PageSize = scudo::getPageSizeCached();
   scudo::ScopedString Str;
+  Str.reserve(2 * PageSize);
   Str.clear();
   fillString(Str, 2 * PageSize);
   Str.clear();
   fillString(Str, PageSize - 64);
   Str.append("%-128s", "12345");
   Str.clear();
   fillString(Str, PageSize - 16);
   Str.append("%024x", 12345);