[Bug] Fixed a string buffer growing strategy

For some reason the growing strategy of asmjit::String was too
aggressive, basically reaching the maximum doubling capacity too
fast (after the first reallocation). This code adapts the current
vector growing strategy to be used also by asmjit::String, which
doubles the capacity until a threshold is reached and then grows
linearly.
This commit is contained in:
kobalicek
2024-06-22 10:12:28 +02:00
parent f5df7a2b1b
commit 062e69ca81
2 changed files with 86 additions and 24 deletions

View File

@@ -3,6 +3,10 @@ on:
push:
pull_request:
concurrency:
group: ${{github.ref}}
cancel-in-progress: ${{github.ref != 'refs/heads/master'}}
defaults:
run:
shell: bash
@@ -133,14 +137,14 @@ jobs:
- { title: "windows" , host: "windows-2022" , arch: "x64" , cc: "vs2022" , conf: "Debug" , defs: "ASMJIT_TEST=1" }
- { title: "windows" , host: "windows-2022" , arch: "x64" , cc: "vs2022" , conf: "Release", defs: "ASMJIT_TEST=1" }
- { title: "freebsd" , host: "macos-12" , arch: "x86-64" , cc: "clang" , conf: "Release", vm: "freebsd", vm_ver: "13.2", defs: "ASMJIT_TEST=1" }
- { title: "netbsd" , host: "macos-12" , arch: "x86-64" , cc: "clang" , conf: "Release", vm: "netbsd" , vm_ver: "9.3" , defs: "ASMJIT_TEST=1" }
- { title: "openbsd" , host: "macos-12" , arch: "x86-64" , cc: "clang" , conf: "Release", vm: "openbsd", vm_ver: "7.4" , defs: "ASMJIT_TEST=1" }
- { title: "freebsd" , host: "ubuntu-latest" , arch: "x64" , cc: "clang" , conf: "Release", vm: "freebsd", vm_ver: "14.1", defs: "ASMJIT_TEST=1" }
- { title: "freebsd" , host: "ubuntu-latest" , arch: "arm64" , cc: "clang" , conf: "Release", vm: "freebsd", vm_ver: "14.1", defs: "ASMJIT_TEST=1" }
- { title: "netbsd" , host: "ubuntu-latest" , arch: "x64" , cc: "clang" , conf: "Release", vm: "netbsd" , vm_ver: "10.0", defs: "ASMJIT_TEST=1" }
- { title: "netbsd" , host: "ubuntu-latest" , arch: "arm64" , cc: "clang" , conf: "Release", vm: "netbsd" , vm_ver: "10.0", defs: "ASMJIT_TEST=1" }
- { title: "openbsd" , host: "ubuntu-latest" , arch: "x64" , cc: "clang" , conf: "Release", vm: "openbsd", vm_ver: "7.4" , defs: "ASMJIT_TEST=1" }
- { title: "openbsd" , host: "ubuntu-latest" , arch: "arm64" , cc: "clang" , conf: "Release", vm: "openbsd", vm_ver: "7.4" , defs: "ASMJIT_TEST=1" }
# arm/v7 VM image doesn't work on CI environment at the moment
# - { title: "debian" , host: "ubuntu-latest" , arch: "arm/v7" , cc: "clang" , conf: "Release", vm: "debian:unstable", defs: "ASMJIT_TEST=1" }
- { title: "debian" , host: "ubuntu-latest" , arch: "arm/v7" , cc: "clang" , conf: "Release", vm: "debian:unstable", defs: "ASMJIT_TEST=1" }
- { title: "debian" , host: "ubuntu-latest" , arch: "arm64" , cc: "clang" , conf: "Release", vm: "debian:unstable", defs: "ASMJIT_TEST=1" }
- { title: "debian" , host: "ubuntu-latest" , arch: "riscv64", cc: "clang" , conf: "Release", vm: "debian:unstable", defs: "ASMJIT_TEST=1" }
- { title: "debian" , host: "ubuntu-latest" , arch: "ppc64le", cc: "clang" , conf: "Release", vm: "debian:unstable", defs: "ASMJIT_TEST=1" }

View File

@@ -14,9 +14,51 @@ ASMJIT_BEGIN_NAMESPACE
static const char String_baseN[] = "0123456789ABCDEF";
constexpr size_t kMinAllocSize = 64;
constexpr size_t kMinAllocSize = 128;
constexpr size_t kMaxAllocSize = SIZE_MAX - Globals::kGrowThreshold;
// Based on ZoneVector_growCapacity().
//
// NOTE: The sizes here include null terminators - that way we can have aligned allocations that are power of 2s
// initially.
static ASMJIT_FORCE_INLINE size_t String_growCapacity(size_t byteSize, size_t minimumByteSize) noexcept {
static constexpr size_t kGrowThreshold = Globals::kGrowThreshold;
ASMJIT_ASSERT(minimumByteSize < kMaxAllocSize);
// This is more than exponential growth at the beginning.
if (byteSize < kMinAllocSize) {
byteSize = kMinAllocSize;
}
else if (byteSize < 512) {
byteSize = 512;
}
if (byteSize < minimumByteSize) {
// Exponential growth before we reach `kGrowThreshold`.
byteSize = Support::alignUpPowerOf2(minimumByteSize);
// Bail to `minimumByteSize` in case of overflow - most likely whatever that is happening afterwards would just fail.
if (byteSize < minimumByteSize) {
return minimumByteSize;
}
// Pretty much chunked growth advancing by `kGrowThreshold` after we exceed it.
if (byteSize > kGrowThreshold) {
// Align to kGrowThreshold.
size_t remainder = minimumByteSize % kGrowThreshold;
byteSize = minimumByteSize + remainder;
// Bail to `minimumByteSize` in case of overflow.
if (byteSize < minimumByteSize)
return minimumByteSize;
}
}
return Support::min<size_t>(byteSize, kMaxAllocSize);
}
// String - Clear & Reset
// ======================
@@ -49,13 +91,13 @@ char* String::prepare(ModifyOp op, size_t size) noexcept {
size_t curCapacity;
if (isLargeOrExternal()) {
curData = this->_large.data;
curSize = this->_large.size;
curCapacity = this->_large.capacity;
curData = _large.data;
curSize = _large.size;
curCapacity = _large.capacity;
}
else {
curData = this->_small.data;
curSize = this->_small.type;
curData = _small.data;
curSize = _small.type;
curCapacity = kSSOCapacity;
}
@@ -90,25 +132,20 @@ char* String::prepare(ModifyOp op, size_t size) noexcept {
}
else {
// Prevent arithmetic overflow.
if (ASMJIT_UNLIKELY(size >= kMaxAllocSize - curSize))
if (ASMJIT_UNLIKELY(size >= kMaxAllocSize - curSize - 1))
return nullptr;
size_t newSize = size + curSize;
size_t newSizePlusOne = newSize + 1;
if (newSizePlusOne > curCapacity) {
size_t newCapacity = Support::max<size_t>(curCapacity + 1, kMinAllocSize);
if (newSize > curCapacity) {
size_t newCapacityPlusOne = String_growCapacity(size + 1u, newSizePlusOne);
ASMJIT_ASSERT(newCapacityPlusOne >= newSizePlusOne);
if (newCapacity < newSizePlusOne && newCapacity < Globals::kGrowThreshold)
newCapacity = Support::alignUpPowerOf2(newCapacity);
if (newCapacity < newSizePlusOne)
newCapacity = Support::alignUp(newSizePlusOne, Globals::kGrowThreshold);
if (ASMJIT_UNLIKELY(newCapacity < newSizePlusOne))
if (ASMJIT_UNLIKELY(newCapacityPlusOne < newSizePlusOne))
return nullptr;
char* newData = static_cast<char*>(::malloc(newCapacity));
char* newData = static_cast<char*>(::malloc(newCapacityPlusOne));
if (ASMJIT_UNLIKELY(!newData))
return nullptr;
@@ -119,7 +156,7 @@ char* String::prepare(ModifyOp op, size_t size) noexcept {
_large.type = kTypeLarge;
_large.size = newSize;
_large.capacity = newCapacity - 1;
_large.capacity = newCapacityPlusOne - 1;
_large.data = newData;
newData[newSize] = '\0';
@@ -488,9 +525,28 @@ bool String::equals(const char* other, size_t size) const noexcept {
// ==============
#if defined(ASMJIT_TEST)
static void test_string_grow() noexcept {
String s;
size_t c = s.capacity();
INFO("Testing string grow strategy (SSO capacity: %zu)", c);
for (size_t i = 0; i < 1000000; i++) {
s.append('x');
if (s.capacity() != c) {
c = s.capacity();
INFO(" String reallocated to new capacity: %zu", c);
}
}
// We don't expect a 1 million character string to occupy 4MiB, for example. So verify that!
EXPECT_LT(c, size_t(4 * 1024 * 1024));
}
UNIT(core_string) {
String s;
INFO("Testing string functionality");
EXPECT_FALSE(s.isLargeOrExternal());
EXPECT_FALSE(s.isExternal());
@@ -553,6 +609,8 @@ UNIT(core_string) {
EXPECT_TRUE(sTmp.isExternal());
EXPECT_EQ(sTmp.appendChars(' ', 1000), kErrorOk);
EXPECT_FALSE(sTmp.isExternal());
test_string_grow();
}
#endif