[libcxx-commits] [libcxx] Lower std::string's alignment requirement from 16 to 8. (PR #68807)
via libcxx-commits
libcxx-commits at lists.llvm.org
Thu Oct 12 16:32:44 PDT 2023
https://github.com/EricWF updated https://github.com/llvm/llvm-project/pull/68807
>From b98e105c69045c11aeba20b096bcdffd4ea8fac7 Mon Sep 17 00:00:00 2001
From: Eric Fiselier <eric at efcs.ca>
Date: Thu, 12 Oct 2023 15:09:18 -0400
Subject: [PATCH 1/5] Lower std::string's alignment requirement from 16 to 8.
This allows smaller allocations to occur, closer to the actual std::string's required size. This is particularly effective in decreasing the allocation size upon initial construction (where __recommend is called to determine the size).
Although the memory savings per-string are never more than 8 bytes per string initially, this quickly adds up. And has lead to not insigficant memory savings at Google.
Unfortunately, this change is ABI breaking because it changes the value returned by max_size. So it has to be guarded.
---
libcxx/include/__config | 5 +++
libcxx/include/string | 9 +++-
.../string.capacity/allocation_size.pass.cpp | 43 +++++++++++++++++++
.../string.capacity/max_size.pass.cpp | 8 +++-
4 files changed, 63 insertions(+), 2 deletions(-)
create mode 100644 libcxx/test/libcxx/strings/basic.string/string.capacity/allocation_size.pass.cpp
diff --git a/libcxx/include/__config b/libcxx/include/__config
index 55d9f1c737652e6..01b1aa74163e4b1 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -167,6 +167,11 @@
// The implementation moved to the header, but we still export the symbols from
// the dylib for backwards compatibility.
# define _LIBCPP_ABI_DO_NOT_EXPORT_TO_CHARS_BASE_10
+// Same memory by providing the allocator more freedom to allocate the most
+// efficient size class by dropping the alignment requirements for std::string's
+// pointer from 16 to 8. This changes the output of std::string::max_size,
+// which makes it ABI breaking
+# define _LIBCPP_ABI_STRING_8_BYTE_ALIGNMENT
# elif _LIBCPP_ABI_VERSION == 1
# if !(defined(_LIBCPP_OBJECT_FORMAT_COFF) || defined(_LIBCPP_OBJECT_FORMAT_XCOFF))
// Enable compiling copies of now inline methods into the dylib to support
diff --git a/libcxx/include/string b/libcxx/include/string
index 33e87406a1156a6..3078715e02b358a 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -1851,7 +1851,14 @@ private:
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20
size_type __align_it(size_type __s) _NOEXCEPT
{return (__s + (__a-1)) & ~(__a-1);}
- enum {__alignment = 16};
+ enum {
+ __alignment =
+#ifdef _LIBCPP_ABI_STRING_8_BYTE_ALIGNMENT
+ 8
+#else
+ 16
+#endif
+ };
static _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20
size_type __recommend(size_type __s) _NOEXCEPT
{
diff --git a/libcxx/test/libcxx/strings/basic.string/string.capacity/allocation_size.pass.cpp b/libcxx/test/libcxx/strings/basic.string/string.capacity/allocation_size.pass.cpp
new file mode 100644
index 000000000000000..6260cbd92e17b11
--- /dev/null
+++ b/libcxx/test/libcxx/strings/basic.string/string.capacity/allocation_size.pass.cpp
@@ -0,0 +1,43 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <string>
+
+// This test demonstrates the smaller allocation sizes when the alignment
+// requirements of std::string are dropped from 16 to 8.
+#include <algorithm>
+#include <cassert>
+#include <cstddef>
+#include <string>
+
+#include "test_macros.h"
+
+// alignment of the string heap buffer is hardcoded to 16
+
+constexpr std::size_t alignment =
+#ifdef _LIBCPP_ABI_STRING_8_BYTE_ALIGNMENT
+ 8;
+#else
+ 16;
+#endif
+
+int main(int, char**) {
+ std::string input_string;
+ input_string.resize(64, 'a');
+
+ // Call a constructor which selects its size using __recommend.
+ std::string test_string(input_string.data());
+ constexpr std::size_t expected_align8_size = 71;
+
+ // Demonstrate the lesser capacity/allocation size when the alignment requirement is 8.
+ if (alignment == 8) {
+ assert(test_string.capacity() == expected_align8_size);
+ } else {
+ assert(test_string.capacity() == expected_align8_size + 8);
+ }
+}
diff --git a/libcxx/test/libcxx/strings/basic.string/string.capacity/max_size.pass.cpp b/libcxx/test/libcxx/strings/basic.string/string.capacity/max_size.pass.cpp
index 5af9cab0be4e80a..a3cb79522f2e1eb 100644
--- a/libcxx/test/libcxx/strings/basic.string/string.capacity/max_size.pass.cpp
+++ b/libcxx/test/libcxx/strings/basic.string/string.capacity/max_size.pass.cpp
@@ -18,7 +18,13 @@
#include "test_macros.h"
// alignment of the string heap buffer is hardcoded to 16
-static const std::size_t alignment = 16;
+
+static const std::size_t alignment =
+#ifdef _LIBCPP_ABI_STRING_8_BYTE_ALIGNMENT
+ 8;
+#else
+ 16;
+#endif
template <class = int>
TEST_CONSTEXPR_CXX20 void full_size() {
>From 734450ce6c5261c41c5aad7ce87923ee5bbb5574 Mon Sep 17 00:00:00 2001
From: Eric Fiselier <eric at efcs.ca>
Date: Thu, 12 Oct 2023 16:39:07 -0400
Subject: [PATCH 2/5] Add release notes as requested.
I added them once before, but it got lost as I messed up the patches.
---
libcxx/docs/ReleaseNotes/18.rst | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/libcxx/docs/ReleaseNotes/18.rst b/libcxx/docs/ReleaseNotes/18.rst
index 5f43d2f2afe22d3..2c92e3c6a003a95 100644
--- a/libcxx/docs/ReleaseNotes/18.rst
+++ b/libcxx/docs/ReleaseNotes/18.rst
@@ -133,6 +133,13 @@ ABI Affecting Changes
results in an ABI break, however in practice we expect uses of ``std::projected`` in ABI-sensitive places to be
extremely rare. Any error resulting from this change should result in a link-time error.
+- Under the unstable ABI, the internal alignment requirements for heap allocations
+ inside std::string has decreased from 16 to 8 This save memory since string requests fewer additional
+ bytes than it did previously. However, this also changes the return value of std::string::max_length
+ and can cause code compiled against older libc++ versions but linked at runtime to a new version
+ to thrown a different exception when attempting allocations that are too large
+ (std::bad_alloc vs std::length_error).
+
Build System Changes
--------------------
>From e9e311ae9a6435ef44d60a93329a646e86102171 Mon Sep 17 00:00:00 2001
From: Eric Fiselier <eric at efcs.ca>
Date: Thu, 12 Oct 2023 17:47:19 -0400
Subject: [PATCH 3/5] Fight with the formatter again.
---
libcxx/include/__config | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libcxx/include/__config b/libcxx/include/__config
index 01b1aa74163e4b1..0063258c6a25b5c 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -171,7 +171,7 @@
// efficient size class by dropping the alignment requirements for std::string's
// pointer from 16 to 8. This changes the output of std::string::max_size,
// which makes it ABI breaking
-# define _LIBCPP_ABI_STRING_8_BYTE_ALIGNMENT
+# define _LIBCPP_ABI_STRING_8_BYTE_ALIGNMENT
# elif _LIBCPP_ABI_VERSION == 1
# if !(defined(_LIBCPP_OBJECT_FORMAT_COFF) || defined(_LIBCPP_OBJECT_FORMAT_XCOFF))
// Enable compiling copies of now inline methods into the dylib to support
>From 06ea65d4ab0895e2629ebc7f7117d793bc76efe8 Mon Sep 17 00:00:00 2001
From: Eric Fiselier <eric at efcs.ca>
Date: Thu, 12 Oct 2023 19:31:18 -0400
Subject: [PATCH 4/5] address review comments
---
libcxx/docs/ReleaseNotes/18.rst | 2 +-
libcxx/include/__config | 2 +-
.../basic.string/string.capacity/allocation_size.pass.cpp | 6 ++++--
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/libcxx/docs/ReleaseNotes/18.rst b/libcxx/docs/ReleaseNotes/18.rst
index 2c92e3c6a003a95..a4ecf56466f48c8 100644
--- a/libcxx/docs/ReleaseNotes/18.rst
+++ b/libcxx/docs/ReleaseNotes/18.rst
@@ -137,7 +137,7 @@ ABI Affecting Changes
inside std::string has decreased from 16 to 8 This save memory since string requests fewer additional
bytes than it did previously. However, this also changes the return value of std::string::max_length
and can cause code compiled against older libc++ versions but linked at runtime to a new version
- to thrown a different exception when attempting allocations that are too large
+ to throw a different exception when attempting allocations that are too large
(std::bad_alloc vs std::length_error).
Build System Changes
diff --git a/libcxx/include/__config b/libcxx/include/__config
index 0063258c6a25b5c..65ce6d6a27f8326 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -167,7 +167,7 @@
// The implementation moved to the header, but we still export the symbols from
// the dylib for backwards compatibility.
# define _LIBCPP_ABI_DO_NOT_EXPORT_TO_CHARS_BASE_10
-// Same memory by providing the allocator more freedom to allocate the most
+// Save memory by providing the allocator more freedom to allocate the most
// efficient size class by dropping the alignment requirements for std::string's
// pointer from 16 to 8. This changes the output of std::string::max_size,
// which makes it ABI breaking
diff --git a/libcxx/test/libcxx/strings/basic.string/string.capacity/allocation_size.pass.cpp b/libcxx/test/libcxx/strings/basic.string/string.capacity/allocation_size.pass.cpp
index 6260cbd92e17b11..97073a6b3a3db16 100644
--- a/libcxx/test/libcxx/strings/basic.string/string.capacity/allocation_size.pass.cpp
+++ b/libcxx/test/libcxx/strings/basic.string/string.capacity/allocation_size.pass.cpp
@@ -19,7 +19,7 @@
// alignment of the string heap buffer is hardcoded to 16
-constexpr std::size_t alignment =
+const std::size_t alignment =
#ifdef _LIBCPP_ABI_STRING_8_BYTE_ALIGNMENT
8;
#else
@@ -32,7 +32,7 @@ int main(int, char**) {
// Call a constructor which selects its size using __recommend.
std::string test_string(input_string.data());
- constexpr std::size_t expected_align8_size = 71;
+ const std::size_t expected_align8_size = 71;
// Demonstrate the lesser capacity/allocation size when the alignment requirement is 8.
if (alignment == 8) {
@@ -40,4 +40,6 @@ int main(int, char**) {
} else {
assert(test_string.capacity() == expected_align8_size + 8);
}
+
+ return 0;
}
>From 6f0284399c3e9018a6333958ad916916dd682e54 Mon Sep 17 00:00:00 2001
From: Eric Fiselier <eric at efcs.ca>
Date: Thu, 12 Oct 2023 19:32:30 -0400
Subject: [PATCH 5/5] Fix another review comment that got reverted
---
.../basic.string/string.capacity/allocation_size.pass.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libcxx/test/libcxx/strings/basic.string/string.capacity/allocation_size.pass.cpp b/libcxx/test/libcxx/strings/basic.string/string.capacity/allocation_size.pass.cpp
index 97073a6b3a3db16..c7df56c815a8054 100644
--- a/libcxx/test/libcxx/strings/basic.string/string.capacity/allocation_size.pass.cpp
+++ b/libcxx/test/libcxx/strings/basic.string/string.capacity/allocation_size.pass.cpp
@@ -17,7 +17,7 @@
#include "test_macros.h"
-// alignment of the string heap buffer is hardcoded to 16
+// alignment of the string heap buffer is hardcoded to either 16 or 8
const std::size_t alignment =
#ifdef _LIBCPP_ABI_STRING_8_BYTE_ALIGNMENT
More information about the libcxx-commits
mailing list