This is an automated email from the ASF dual-hosted git repository.
tqchen pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tvm-ffi.git
The following commit(s) were added to refs/heads/main by this push:
new 18afa9c8 [FIX] Construct small moved strings and bytes inline (#898)
18afa9c8 is described below
commit 18afa9c8ac90467b635171d208000ef5992471e4
Author: Tianqi Chen <[email protected]>
AuthorDate: Mon Nov 2 17:16:15 2026 -0500
[FIX] Construct small moved strings and bytes inline (#898)
Use inline storage when constructing `String` or `Bytes` from a small
`std::string` rvalue. Share the existing small-value capacity check and
retain the moved heap representation for larger inputs.
Existing externally supplied heap objects retain their representation.
---
include/tvm/ffi/string.h | 16 ++++++++++++----
tests/cpp/test_any.cc | 6 +++---
tests/cpp/test_string.cc | 27 +++++++++++++++++++++++++++
3 files changed, 42 insertions(+), 7 deletions(-)
diff --git a/include/tvm/ffi/string.h b/include/tvm/ffi/string.h
index a830d581..69fa37f0 100644
--- a/include/tvm/ffi/string.h
+++ b/include/tvm/ffi/string.h
@@ -185,7 +185,13 @@ class BytesBaseCell {
}
template <typename LargeObj>
- void InitFromStd(std::string&& other, int32_t large_type_index) {
+ void InitFromStd(std::string&& other, int32_t small_type_index, int32_t
large_type_index) {
+ if (other.size() <= kMaxSmallBytesLen) {
+ char* dest_data =
+ InitSpaceForSize<LargeObj>(other.size(), small_type_index,
large_type_index);
+ std::memcpy(dest_data, other.c_str(), other.size() + 1);
+ return;
+ }
// needs to be reset to none first for exception safety
data_.type_index = TypeIndex::kTVMFFINone;
data_.zero_padding = 0;
@@ -205,7 +211,6 @@ class BytesBaseCell {
*/
template <typename LargeObj>
char* InitSpaceForSize(size_t size, int32_t small_type_index, int32_t
large_type_index) {
- size_t kMaxSmallBytesLen = sizeof(int64_t) - 1;
// first zero the content, this is important for exception safety
data_.type_index = small_type_index;
data_.zero_padding = 0;
@@ -255,6 +260,7 @@ class BytesBaseCell {
}
private:
+ static constexpr size_t kMaxSmallBytesLen = sizeof(int64_t) - 1;
explicit BytesBaseCell(TVMFFIAny data) : data_(data) {}
/*! \brief internal backing data */
TVMFFIAny data_;
@@ -297,7 +303,8 @@ class Bytes {
* \param other a char array.
*/
Bytes(std::string&& other) { // NOLINT(*)
- data_.InitFromStd<detail::BytesObj>(std::move(other),
TypeIndex::kTVMFFIBytes);
+ data_.InitFromStd<detail::BytesObj>(std::move(other),
TypeIndex::kTVMFFISmallBytes,
+ TypeIndex::kTVMFFIBytes);
}
/*!
* \brief Swap this String with another string
@@ -498,7 +505,8 @@ class String {
String(std::string&& other) { // NOLINT(*)
// exception safety, first set to none so if exception is thrown
// destructor works correctly
- data_.InitFromStd<detail::StringObj>(std::move(other),
TypeIndex::kTVMFFIStr);
+ data_.InitFromStd<detail::StringObj>(std::move(other),
TypeIndex::kTVMFFISmallStr,
+ TypeIndex::kTVMFFIStr);
}
/*!
diff --git a/tests/cpp/test_any.cc b/tests/cpp/test_any.cc
index 73ef6de1..16b753fa 100644
--- a/tests/cpp/test_any.cc
+++ b/tests/cpp/test_any.cc
@@ -558,15 +558,15 @@ TEST(Any, ObjectMove) {
TEST(Any, AnyEqualHash) {
// small string
Any a = "a1";
- // on heap allocated string
- Any b = String(std::string("a1"));
+ // Explicit heap objects retain their representation, even for small
contents.
+ Any b = make_object<detail::BytesObjStdImpl<detail::StringObj>>("a1");
EXPECT_EQ(a.type_index(), TypeIndex::kTVMFFISmallStr);
EXPECT_EQ(b.type_index(), TypeIndex::kTVMFFIStr);
EXPECT_TRUE(AnyEqual()(a, b));
EXPECT_EQ(AnyHash()(a), AnyHash()(b));
Any c = Bytes("a1", 2);
- Any d = Bytes(std::string("a1"));
+ Any d = make_object<detail::BytesObjStdImpl<detail::BytesObj>>("a1");
EXPECT_EQ(c.type_index(), TypeIndex::kTVMFFISmallBytes);
EXPECT_EQ(d.type_index(), TypeIndex::kTVMFFIBytes);
EXPECT_TRUE(AnyEqual()(c, d));
diff --git a/tests/cpp/test_string.cc b/tests/cpp/test_string.cc
index 6df13cfe..38db118b 100644
--- a/tests/cpp/test_string.cc
+++ b/tests/cpp/test_string.cc
@@ -20,6 +20,8 @@
#include <tvm/ffi/any.h>
#include <tvm/ffi/string.h>
+#include <cstdint>
+
namespace {
using namespace tvm::ffi;
@@ -34,6 +36,31 @@ TEST(String, MoveFromStd) {
EXPECT_EQ(source.size(), 0); // NOLINT(bugprone-use-after-move)
}
+TEST(String, MoveFromStdStorage) {
+ for (const std::string& expected : {std::string(), std::string("a\0b", 3),
std::string(7, 'x'),
+ std::string(8, 'x'), std::string(64,
'x')}) {
+ std::string string_source = expected;
+ std::string bytes_source = expected;
+ const auto string_address =
reinterpret_cast<std::uintptr_t>(string_source.data());
+ const auto bytes_address =
reinterpret_cast<std::uintptr_t>(bytes_source.data());
+ String str(std::move(string_source));
+ Bytes bytes(std::move(bytes_source));
+ const bool small = expected.size() <= 7;
+ EXPECT_EQ(AnyView(str).type_index(),
+ small ? TypeIndex::kTVMFFISmallStr : TypeIndex::kTVMFFIStr);
+ EXPECT_EQ(AnyView(bytes).type_index(),
+ small ? TypeIndex::kTVMFFISmallBytes : TypeIndex::kTVMFFIBytes);
+ EXPECT_EQ(std::string(str), expected);
+ EXPECT_EQ(std::string(bytes), expected);
+ EXPECT_EQ(str.data()[str.size()], '\0');
+ EXPECT_EQ(bytes.data()[bytes.size()], '\0');
+ if (expected.size() == 64) {
+ EXPECT_EQ(reinterpret_cast<std::uintptr_t>(str.data()), string_address);
+ EXPECT_EQ(reinterpret_cast<std::uintptr_t>(bytes.data()), bytes_address);
+ }
+ }
+}
+
TEST(String, CopyFromStd) {
using namespace std;
string source = "this is a string";
