[compiler-rt] 868317b - [scudo] Rework Vector/String
Kostya Kortchinsky via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 3 18:12:58 PDT 2021
Author: Kostya Kortchinsky
Date: 2021-06-03T18:12:24-07:00
New Revision: 868317b3fd765830c07ecf16cbfcf6c7708adc3c
URL: https://github.com/llvm/llvm-project/commit/868317b3fd765830c07ecf16cbfcf6c7708adc3c
DIFF: https://github.com/llvm/llvm-project/commit/868317b3fd765830c07ecf16cbfcf6c7708adc3c.diff
LOG: [scudo] Rework Vector/String
Some platforms (eg: Trusty) are extremelly memory constrained, which
doesn't necessarily work well with some of Scudo's current assumptions.
`Vector` by default (and as such `String` and `ScopedString`) maps a
page, which is a bit of a waste. This CL changes `Vector` to use a
buffer local to the class first, then potentially map more memory if
needed (`ScopedString` currently are all stack based so it would be
stack data). We also want to allow a platform to prevent any dynamic
resizing, so I added a `CanGrow` templated parameter that for now is
always `true` but would be set to `false` on Trusty.
Differential Revision: https://reviews.llvm.org/D103641
Added:
Modified:
compiler-rt/lib/scudo/standalone/combined.h
compiler-rt/lib/scudo/standalone/primary64.h
compiler-rt/lib/scudo/standalone/report.cpp
compiler-rt/lib/scudo/standalone/size_class_map.h
compiler-rt/lib/scudo/standalone/string_utils.cpp
compiler-rt/lib/scudo/standalone/string_utils.h
compiler-rt/lib/scudo/standalone/tests/primary_test.cpp
compiler-rt/lib/scudo/standalone/tests/quarantine_test.cpp
compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp
compiler-rt/lib/scudo/standalone/tests/strings_test.cpp
compiler-rt/lib/scudo/standalone/tests/vector_test.cpp
compiler-rt/lib/scudo/standalone/vector.h
Removed:
################################################################################
diff --git a/compiler-rt/lib/scudo/standalone/combined.h b/compiler-rt/lib/scudo/standalone/combined.h
index 8caffcc533c0..f58eaa945af3 100644
--- a/compiler-rt/lib/scudo/standalone/combined.h
+++ b/compiler-rt/lib/scudo/standalone/combined.h
@@ -694,7 +694,7 @@ class Allocator {
// function. This can be called with a null buffer or zero size for buffer
// sizing purposes.
uptr getStats(char *Buffer, uptr Size) {
- ScopedString Str(1024);
+ ScopedString Str;
disable();
const uptr Length = getStats(&Str) + 1;
enable();
@@ -708,7 +708,7 @@ class Allocator {
}
void printStats() {
- ScopedString Str(1024);
+ ScopedString Str;
disable();
getStats(&Str);
enable();
diff --git a/compiler-rt/lib/scudo/standalone/primary64.h b/compiler-rt/lib/scudo/standalone/primary64.h
index 0a43f46bb2c7..a3456b6b99c1 100644
--- a/compiler-rt/lib/scudo/standalone/primary64.h
+++ b/compiler-rt/lib/scudo/standalone/primary64.h
@@ -341,7 +341,7 @@ template <typename Config> class SizeClassAllocator64 {
if (UNLIKELY(RegionBase + MappedUser + MapSize > RegionSize)) {
if (!Region->Exhausted) {
Region->Exhausted = true;
- ScopedString Str(1024);
+ ScopedString Str;
getStats(&Str);
Str.append(
"Scudo OOM: The process has exhausted %zuM for size class %zu.\n",
diff --git a/compiler-rt/lib/scudo/standalone/report.cpp b/compiler-rt/lib/scudo/standalone/report.cpp
index 292c29918d1a..561c7c51f4e1 100644
--- a/compiler-rt/lib/scudo/standalone/report.cpp
+++ b/compiler-rt/lib/scudo/standalone/report.cpp
@@ -17,7 +17,7 @@ namespace scudo {
class ScopedErrorReport {
public:
- ScopedErrorReport() : Message(512) { Message.append("Scudo ERROR: "); }
+ ScopedErrorReport() : Message() { Message.append("Scudo ERROR: "); }
void append(const char *Format, ...) {
va_list Args;
va_start(Args, Format);
diff --git a/compiler-rt/lib/scudo/standalone/size_class_map.h b/compiler-rt/lib/scudo/standalone/size_class_map.h
index 2736e995dbab..e1c2edac4763 100644
--- a/compiler-rt/lib/scudo/standalone/size_class_map.h
+++ b/compiler-rt/lib/scudo/standalone/size_class_map.h
@@ -309,7 +309,7 @@ struct SvelteSizeClassConfig {
typedef FixedSizeClassMap<SvelteSizeClassConfig> SvelteSizeClassMap;
template <typename SCMap> inline void printMap() {
- ScopedString Buffer(1024);
+ ScopedString Buffer;
uptr PrevS = 0;
uptr TotalCached = 0;
for (uptr I = 0; I < SCMap::NumClasses; I++) {
diff --git a/compiler-rt/lib/scudo/standalone/string_utils.cpp b/compiler-rt/lib/scudo/standalone/string_utils.cpp
index 25bddbce34d9..ca80834cca87 100644
--- a/compiler-rt/lib/scudo/standalone/string_utils.cpp
+++ b/compiler-rt/lib/scudo/standalone/string_utils.cpp
@@ -219,7 +219,7 @@ int formatString(char *Buffer, uptr BufferLength, const char *Format, ...) {
}
void ScopedString::append(const char *Format, va_list Args) {
- DCHECK_LT(Length, String.size());
+ RAW_CHECK(Length <= String.size());
va_list ArgsCopy;
va_copy(ArgsCopy, Args);
// formatString doesn't currently support a null buffer or zero buffer length,
@@ -232,7 +232,7 @@ void ScopedString::append(const char *Format, va_list Args) {
formatString(String.data() + Length, AdditionalLength, Format, ArgsCopy);
va_end(ArgsCopy);
Length = strlen(String.data());
- CHECK_LT(Length, String.size());
+ RAW_CHECK(Length < String.size());
}
FORMAT(2, 3)
@@ -247,7 +247,7 @@ FORMAT(1, 2)
void Printf(const char *Format, ...) {
va_list Args;
va_start(Args, Format);
- ScopedString Msg(1024);
+ ScopedString Msg;
Msg.append(Format, Args);
outputRaw(Msg.data());
va_end(Args);
diff --git a/compiler-rt/lib/scudo/standalone/string_utils.h b/compiler-rt/lib/scudo/standalone/string_utils.h
index 4880fa1e7cf1..7a57da3a53f4 100644
--- a/compiler-rt/lib/scudo/standalone/string_utils.h
+++ b/compiler-rt/lib/scudo/standalone/string_utils.h
@@ -18,8 +18,9 @@ namespace scudo {
class ScopedString {
public:
- explicit ScopedString(uptr MaxLength) : String(MaxLength), Length(0) {
- String[0] = '\0';
+ explicit ScopedString() : String() {
+ if (String.capacity() > 0)
+ String[0] = '\0';
}
uptr length() { return Length; }
const char *data() { return String.data(); }
@@ -33,7 +34,7 @@ class ScopedString {
private:
Vector<char> String;
- uptr Length;
+ uptr Length = 0;
};
int formatString(char *Buffer, uptr BufferLength, const char *Format, ...);
diff --git a/compiler-rt/lib/scudo/standalone/tests/primary_test.cpp b/compiler-rt/lib/scudo/standalone/tests/primary_test.cpp
index 3574c28c6c09..93157f942c54 100644
--- a/compiler-rt/lib/scudo/standalone/tests/primary_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/primary_test.cpp
@@ -132,7 +132,7 @@ SCUDO_TYPED_TEST(ScudoPrimaryTest, BasicPrimary) {
}
Cache.destroy(nullptr);
Allocator->releaseToOS();
- scudo::ScopedString Str(1024);
+ scudo::ScopedString Str;
Allocator->getStats(&Str);
Str.output();
}
@@ -178,7 +178,7 @@ TEST(ScudoPrimaryTest, Primary64OOM) {
}
Cache.destroy(nullptr);
Allocator.releaseToOS();
- scudo::ScopedString Str(1024);
+ scudo::ScopedString Str;
Allocator.getStats(&Str);
Str.output();
EXPECT_EQ(AllocationFailed, true);
@@ -216,7 +216,7 @@ SCUDO_TYPED_TEST(ScudoPrimaryTest, PrimaryIterate) {
}
Cache.destroy(nullptr);
Allocator->releaseToOS();
- scudo::ScopedString Str(1024);
+ scudo::ScopedString Str;
Allocator->getStats(&Str);
Str.output();
}
@@ -263,7 +263,7 @@ SCUDO_TYPED_TEST(ScudoPrimaryTest, PrimaryThreaded) {
for (auto &T : Threads)
T.join();
Allocator->releaseToOS();
- scudo::ScopedString Str(1024);
+ scudo::ScopedString Str;
Allocator->getStats(&Str);
Str.output();
}
diff --git a/compiler-rt/lib/scudo/standalone/tests/quarantine_test.cpp b/compiler-rt/lib/scudo/standalone/tests/quarantine_test.cpp
index 91de56a78c97..972c98d510a9 100644
--- a/compiler-rt/lib/scudo/standalone/tests/quarantine_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/quarantine_test.cpp
@@ -214,7 +214,7 @@ TEST(ScudoQuarantineTest, GlobalQuarantine) {
Quarantine.drainAndRecycle(&Cache, Cb);
EXPECT_EQ(Cache.getSize(), 0UL);
- scudo::ScopedString Str(1024);
+ scudo::ScopedString Str;
Quarantine.getStats(&Str);
Str.output();
}
@@ -246,7 +246,7 @@ TEST(ScudoQuarantineTest, ThreadedGlobalQuarantine) {
for (scudo::uptr I = 0; I < NumberOfThreads; I++)
pthread_join(T[I].Thread, 0);
- scudo::ScopedString Str(1024);
+ scudo::ScopedString Str;
Quarantine.getStats(&Str);
Str.output();
diff --git a/compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp b/compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp
index 2320a4d546ab..2dc041b94a8c 100644
--- a/compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp
@@ -64,7 +64,7 @@ template <typename Config> static void testSecondaryBasic(void) {
L->deallocate(scudo::Options{}, V.back());
V.pop_back();
}
- scudo::ScopedString Str(1024);
+ scudo::ScopedString Str;
L->getStats(&Str);
Str.output();
L->unmapTestOnly();
@@ -122,7 +122,7 @@ TEST(ScudoSecondaryTest, SecondaryCombinations) {
}
}
}
- scudo::ScopedString Str(1024);
+ scudo::ScopedString Str;
L->getStats(&Str);
Str.output();
L->unmapTestOnly();
@@ -146,7 +146,7 @@ TEST(ScudoSecondaryTest, SecondaryIterate) {
L->deallocate(scudo::Options{}, V.back());
V.pop_back();
}
- scudo::ScopedString Str(1024);
+ scudo::ScopedString Str;
L->getStats(&Str);
Str.output();
L->unmapTestOnly();
@@ -217,7 +217,7 @@ TEST(ScudoSecondaryTest, SecondaryThreadsRace) {
}
for (auto &T : Threads)
T.join();
- scudo::ScopedString Str(1024);
+ scudo::ScopedString Str;
L->getStats(&Str);
Str.output();
L->unmapTestOnly();
diff --git a/compiler-rt/lib/scudo/standalone/tests/strings_test.cpp b/compiler-rt/lib/scudo/standalone/tests/strings_test.cpp
index eed174dc586a..332bac2f796e 100644
--- a/compiler-rt/lib/scudo/standalone/tests/strings_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/strings_test.cpp
@@ -13,7 +13,7 @@
#include <limits.h>
TEST(ScudoStringsTest, Basic) {
- scudo::ScopedString Str(128);
+ scudo::ScopedString Str;
Str.append("a%db%zdc%ue%zuf%xh%zxq%pe%sr", static_cast<int>(-1),
static_cast<scudo::uptr>(-2), static_cast<unsigned>(-4),
static_cast<scudo::uptr>(5), static_cast<unsigned>(10),
@@ -29,7 +29,7 @@ TEST(ScudoStringsTest, Basic) {
}
TEST(ScudoStringsTest, Precision) {
- scudo::ScopedString Str(128);
+ scudo::ScopedString Str;
Str.append("%.*s", 3, "12345");
EXPECT_EQ(Str.length(), strlen(Str.data()));
EXPECT_STREQ("123", Str.data());
@@ -52,7 +52,7 @@ 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(PageSize);
+ scudo::ScopedString Str;
Str.clear();
fillString(Str, 2 * PageSize);
Str.clear();
@@ -68,7 +68,7 @@ TEST(ScudoStringTest, PotentialOverflows) {
template <typename T>
static void testAgainstLibc(const char *Format, T Arg1, T Arg2) {
- scudo::ScopedString Str(128);
+ scudo::ScopedString Str;
Str.append(Format, Arg1, Arg2);
char Buffer[128];
snprintf(Buffer, sizeof(Buffer), Format, Arg1, Arg2);
diff --git a/compiler-rt/lib/scudo/standalone/tests/vector_test.cpp b/compiler-rt/lib/scudo/standalone/tests/vector_test.cpp
index d2c6a9b6bb3c..dc23c2a34713 100644
--- a/compiler-rt/lib/scudo/standalone/tests/vector_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/vector_test.cpp
@@ -23,14 +23,14 @@ TEST(ScudoVectorTest, Basic) {
}
TEST(ScudoVectorTest, Stride) {
- scudo::Vector<int> V;
- for (int i = 0; i < 1000; i++) {
- V.push_back(i);
- EXPECT_EQ(V.size(), i + 1U);
- EXPECT_EQ(V[i], i);
+ scudo::Vector<scudo::uptr> V;
+ for (scudo::uptr I = 0; I < 1000; I++) {
+ V.push_back(I);
+ EXPECT_EQ(V.size(), I + 1U);
+ EXPECT_EQ(V[I], I);
}
- for (int i = 0; i < 1000; i++)
- EXPECT_EQ(V[i], i);
+ for (scudo::uptr I = 0; I < 1000; I++)
+ EXPECT_EQ(V[I], I);
}
TEST(ScudoVectorTest, ResizeReduction) {
diff --git a/compiler-rt/lib/scudo/standalone/vector.h b/compiler-rt/lib/scudo/standalone/vector.h
index 6ca350a25771..2c9a6e2aa655 100644
--- a/compiler-rt/lib/scudo/standalone/vector.h
+++ b/compiler-rt/lib/scudo/standalone/vector.h
@@ -19,14 +19,13 @@ namespace scudo {
// small vectors. The current implementation supports only POD types.
template <typename T> class VectorNoCtor {
public:
- void init(uptr InitialCapacity) {
- CapacityBytes = 0;
- Size = 0;
- Data = nullptr;
+ void init(uptr InitialCapacity = 0) {
+ Data = reinterpret_cast<T *>(&LocalData[0]);
+ CapacityBytes = sizeof(LocalData);
reserve(InitialCapacity);
}
void destroy() {
- if (Data)
+ if (Data != reinterpret_cast<T *>(&LocalData[0]))
unmap(Data, CapacityBytes);
}
T &operator[](uptr I) {
@@ -82,26 +81,24 @@ template <typename T> class VectorNoCtor {
void reallocate(uptr NewCapacity) {
DCHECK_GT(NewCapacity, 0);
DCHECK_LE(Size, NewCapacity);
- const uptr NewCapacityBytes =
- roundUpTo(NewCapacity * sizeof(T), getPageSizeCached());
+ NewCapacity = roundUpTo(NewCapacity * sizeof(T), getPageSizeCached());
T *NewData =
- reinterpret_cast<T *>(map(nullptr, NewCapacityBytes, "scudo:vector"));
- if (Data) {
- memcpy(NewData, Data, Size * sizeof(T));
- unmap(Data, CapacityBytes);
- }
+ reinterpret_cast<T *>(map(nullptr, NewCapacity, "scudo:vector"));
+ memcpy(NewData, Data, Size * sizeof(T));
+ destroy();
Data = NewData;
- CapacityBytes = NewCapacityBytes;
+ CapacityBytes = NewCapacity;
}
- T *Data;
- uptr CapacityBytes;
- uptr Size;
+ T *Data = nullptr;
+ u8 LocalData[256] = {};
+ uptr CapacityBytes = 0;
+ uptr Size = 0;
};
template <typename T> class Vector : public VectorNoCtor<T> {
public:
- Vector() { VectorNoCtor<T>::init(1); }
+ Vector() { VectorNoCtor<T>::init(); }
explicit Vector(uptr Count) {
VectorNoCtor<T>::init(Count);
this->resize(Count);
More information about the llvm-commits
mailing list