Skip to content

Fix pooled vector reuse for 2.1.1 #11

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dallison merged 1 commit into from
Sep 30, 2026
Merged

Fix pooled vector reuse for 2.1.1 #11

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 2 additions & 10 deletions MODULE.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module(
name = "phaser",
version = "2.1.0"
version = "2.1.1"
)

bazel_dep(name = "bazel_skylib", version = "1.9.0")
Expand All @@ -17,17 +17,9 @@ bazel_dep(name = "rules_shell", version = "0.8.0")
bazel_dep(name = "rules_pkg", version = "1.0.1")
bazel_dep(name = "zlib", version = "1.3.1.bcr.5")

bazel_dep(name = "cpp_toolbelt", version = "2.1.2")
bazel_dep(name = "cpp_toolbelt", version = "2.1.5")
bazel_dep(name = "coroutines", version = "3.3.2")

# Until cpp_toolbelt 2.1.3 is published to BCR, pin the bounds-checking fixes
# from the bounds_fixes branch. Remove this override once the release lands.
git_override(
module_name = "cpp_toolbelt",
remote = "https://github.com/dallison/cpp_toolbelt.git",
commit = "f69cb51",
)

# protobuf pulls an older rules_go via gazelle; Bazel 9 needs a current rules_go.
single_version_override(
module_name = "rules_go",
Expand Down
2 changes: 2 additions & 0 deletions MODULE.bazel.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

34 changes: 20 additions & 14 deletions phaser/runtime/fields.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ class StringField : public Field, public StringFieldFacade<StringField> {
StringField& operator=(const char* s) {
::toolbelt::PayloadBuffer::SetString(
GetBufferAddr(), std::string_view(s, std::strlen(s)),
GetMessageBinaryStart() + relative_binary_offset_);
StringHeaderOffset());
return *this;
}

Expand Down Expand Up @@ -679,34 +679,31 @@ class StringField : public Field, public StringFieldFacade<StringField> {

template <typename Str>
void Set(Str s) {
::toolbelt::PayloadBuffer::SetString(
GetBufferAddr(), s, GetMessageBinaryStart() + relative_binary_offset_);
::toolbelt::PayloadBuffer::SetString(GetBufferAddr(), s,
StringHeaderOffset());
}

void Set(const char* data, size_t size) {
::toolbelt::PayloadBuffer::SetString(
GetBufferAddr(), std::string_view(data, size),
GetMessageBinaryStart() + relative_binary_offset_);
GetBufferAddr(), std::string_view(data, size), StringHeaderOffset());
}

void SetNoCopy(const void* data) {
toolbelt::StringHeader* header =
GetRuntime()->ToAddress<toolbelt::StringHeader>(
GetMessageBinaryStart() + relative_binary_offset_);
GetRuntime()->ToAddress<toolbelt::StringHeader>(StringHeaderOffset());
*header = GetRuntime()->ToOffset(data);
}

void Clear() {
::toolbelt::PayloadBuffer::ClearString(
GetBufferAddr(), GetMessageBinaryStart() + relative_binary_offset_);
::toolbelt::PayloadBuffer::ClearString(GetBufferAddr(), StringHeaderOffset());
}

// Allocate space for the given size for the string and return the
// starting address.
absl::Span<char> Allocate(size_t size, bool clear = false) {
return ::toolbelt::PayloadBuffer::AllocateString(
GetBufferAddr(), size,
GetMessageBinaryStart() + relative_binary_offset_, clear);
return ::toolbelt::PayloadBuffer::AllocateString(GetBufferAddr(), size,
StringHeaderOffset(),
clear);
}

size_t size() const {
Expand Down Expand Up @@ -744,8 +741,8 @@ class StringField : public Field, public StringFieldFacade<StringField> {
if (!s.ok()) {
return s.status();
}
::toolbelt::PayloadBuffer::SetString(
GetBufferAddr(), *s, GetMessageBinaryStart() + relative_binary_offset_);
::toolbelt::PayloadBuffer::SetString(GetBufferAddr(), *s,
StringHeaderOffset());
return absl::OkStatus();
}

Expand All @@ -768,6 +765,15 @@ class StringField : public Field, public StringFieldFacade<StringField> {
return Message::GetMessageBinaryStart(this, source_offset_);
}

::toolbelt::BufferOffset StringHeaderOffset() const {
int32_t offset = FindFieldOffset(source_offset_);
if (offset < 0) {
return GetMessageBinaryStart() + relative_binary_offset_;
}
return GetMessageBinaryStart() +
static_cast<::toolbelt::BufferOffset>(offset);
}

uint32_t source_offset_;
::toolbelt::BufferOffset relative_binary_offset_;
};
Expand Down
12 changes: 4 additions & 8 deletions phaser/runtime/message.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -472,18 +472,14 @@ struct MessageRuntime {
// an underflow read.
size_t AllocatedCapacity(toolbelt::BufferOffset data_offset,
size_t elem_size) const {
if (elem_size == 0 ||
data_offset < sizeof(toolbelt::BufferOffset)) {
if (elem_size == 0 || data_offset == 0 || pb == nullptr) {
return 0;
}
const toolbelt::BufferOffset size_offset =
data_offset -
static_cast<toolbelt::BufferOffset>(sizeof(toolbelt::BufferOffset));
const auto* size_word = ToAddress<const toolbelt::BufferOffset>(size_offset);
if (size_word == nullptr) {
uint32_t* block = pb->ToAddress<uint32_t>(data_offset);
if (block == nullptr) {
return 0;
}
return *size_word / elem_size;
return ::toolbelt::PayloadBuffer::DecodeSize(block) / elem_size;
}

template <typename T = void>
Expand Down
60 changes: 38 additions & 22 deletions phaser/runtime/vectors.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1197,7 +1197,7 @@ class MessageVectorField : public Field {
::toolbelt::BufferOffset absolute_binary_offset =
GetRuntime()->ToOffset(binary);
::toolbelt::PayloadBuffer::VectorPush<::toolbelt::BufferOffset>(
GetBufferAddr(), Header(), absolute_binary_offset);
GetBufferAddr(), ResolvedVectorHeader(), absolute_binary_offset);
T result(GetRuntime(), absolute_binary_offset);
result.template InstallMetadata<T>();
return result;
Expand All @@ -1208,10 +1208,10 @@ class MessageVectorField : public Field {
T Mutable(size_t index) {
if (index >= size()) {
::toolbelt::PayloadBuffer::VectorResize<::toolbelt::BufferOffset>(
GetBufferAddr(), Header(), index + 1);
GetBufferAddr(), ResolvedVectorHeader(), index + 1);
}

auto hdr = Header();
auto hdr = ResolvedVectorHeader();
::toolbelt::BufferOffset* data =
GetRuntime()->template ToAddress<::toolbelt::BufferOffset>(hdr->data);
if (data[index] == 0) {
Expand All @@ -1220,7 +1220,7 @@ class MessageVectorField : public Field {
::toolbelt::BufferOffset absolute_binary_offset =
GetRuntime()->ToOffset(binary);

hdr = Header();
hdr = ResolvedVectorHeader();
data =
GetRuntime()->template ToAddress<::toolbelt::BufferOffset>(hdr->data);
data[index] = absolute_binary_offset;
Expand All @@ -1234,9 +1234,9 @@ class MessageVectorField : public Field {
void SetOffset(int index, toolbelt::BufferOffset offset) {
if (static_cast<size_t>(index) >= size()) {
::toolbelt::PayloadBuffer::VectorResize<::toolbelt::BufferOffset>(
GetBufferAddr(), Header(), static_cast<size_t>(index) + 1);
GetBufferAddr(), ResolvedVectorHeader(), static_cast<size_t>(index) + 1);
}
auto hdr = Header();
auto hdr = ResolvedVectorHeader();
::toolbelt::BufferOffset* data =
GetRuntime()->template ToAddress<::toolbelt::BufferOffset>(hdr->data);
if (data[index] != 0) {
Expand All @@ -1253,12 +1253,12 @@ class MessageVectorField : public Field {
result.reserve(n);
Clear();
::toolbelt::PayloadBuffer::VectorResize<::toolbelt::BufferOffset>(
GetBufferAddr(), Header(), n);
GetBufferAddr(), ResolvedVectorHeader(), n);
// Allocate memory for n messages in the payload buffer.
std::vector<void*> addrs = ::toolbelt::PayloadBuffer::AllocateMany(
GetBufferAddr(), T::BinarySize(), static_cast<uint32_t>(n), true);

toolbelt::VectorHeader* hdr = Header();
toolbelt::VectorHeader* hdr = ResolvedVectorHeader();
::toolbelt::BufferOffset* data =
GetRuntime()->template ToAddress<::toolbelt::BufferOffset>(hdr->data);

Expand All @@ -1279,14 +1279,14 @@ class MessageVectorField : public Field {

void reserve(size_t n) {
::toolbelt::PayloadBuffer::VectorReserve<::toolbelt::BufferOffset>(
GetBufferAddr(), Header(), n);
GetBufferAddr(), ResolvedVectorHeader(), n);
}

void resize(size_t n) {
const size_t old_size = size();
if (n < old_size) {
for (size_t i = n; i < old_size; ++i) {
auto hdr = Header();
auto hdr = ResolvedVectorHeader();
auto data = GetRuntime()->template ToAddress<::toolbelt::BufferOffset>(
hdr->data);
const ::toolbelt::BufferOffset offset = data[i];
Expand All @@ -1297,7 +1297,7 @@ class MessageVectorField : public Field {
}
}
::toolbelt::PayloadBuffer::VectorResize<::toolbelt::BufferOffset>(
GetBufferAddr(), Header(), n);
GetBufferAddr(), ResolvedVectorHeader(), n);
for (size_t i = old_size; i < n; ++i) {
(void)Mutable(i);
}
Expand Down Expand Up @@ -1370,7 +1370,7 @@ class MessageVectorField : public Field {
}

void Clear() {
auto hdr = Header();
auto hdr = ResolvedVectorHeader();
auto data =
GetRuntime()->template ToAddress<::toolbelt::BufferOffset>(hdr->data);
for (uint32_t i = 0; i < hdr->num_elements; i++) {
Expand All @@ -1381,7 +1381,7 @@ class MessageVectorField : public Field {
GetBuffer()->Free(GetRuntime()->ToAddress(data[i]));
}
::toolbelt::PayloadBuffer::VectorClear<::toolbelt::BufferOffset>(
GetBufferAddr(), Header());
GetBufferAddr(), ResolvedVectorHeader());
}
void clear() { Clear(); }

Expand Down Expand Up @@ -1548,6 +1548,14 @@ class MessageVectorField : public Field {
GetMessageBinaryStart() + relative_offset);
}

toolbelt::VectorHeader* ResolvedVectorHeader() const {
int32_t offset = FindFieldOffset(source_offset_);
if (offset < 0) {
offset = static_cast<int32_t>(relative_binary_offset_);
}
return Header(static_cast<uint32_t>(offset));
}

::toolbelt::BufferOffset BaseOffset() const {
int32_t offset = FindFieldOffset(source_offset_);
if (offset < 0) {
Expand Down Expand Up @@ -1806,7 +1814,7 @@ class StringVectorField : public Field {

// Add an offset for the new string to the binary.
::toolbelt::PayloadBuffer::VectorPush<::toolbelt::BufferOffset>(
GetBufferAddr(), Header(), hdr_offset);
GetBufferAddr(), ResolvedVectorHeader(), hdr_offset);
}

template <typename Str>
Expand All @@ -1833,10 +1841,10 @@ class StringVectorField : public Field {
void Set(size_t index, Str s) {
if (index >= size()) {
::toolbelt::PayloadBuffer::VectorResize<::toolbelt::BufferOffset>(
GetBufferAddr(), Header(), index + 1);
GetBufferAddr(), ResolvedVectorHeader(), index + 1);
}

auto hdr = Header();
auto hdr = ResolvedVectorHeader();
::toolbelt::BufferOffset* data =
GetRuntime()->template ToAddress<::toolbelt::BufferOffset>(hdr->data);
if (data[index] == 0) {
Expand All @@ -1845,7 +1853,7 @@ class StringVectorField : public Field {
GetBufferAddr(), sizeof(toolbelt::StringHeader));
::toolbelt::BufferOffset hdr_offset = GetRuntime()->ToOffset(str_hdr);

hdr = Header();
hdr = ResolvedVectorHeader();
data =
GetRuntime()->template ToAddress<::toolbelt::BufferOffset>(hdr->data);
data[index] = hdr_offset;
Expand All @@ -1862,14 +1870,14 @@ class StringVectorField : public Field {

void reserve(size_t n) {
::toolbelt::PayloadBuffer::VectorReserve<::toolbelt::BufferOffset>(
GetBufferAddr(), Header(), n);
GetBufferAddr(), ResolvedVectorHeader(), n);
}

void resize(size_t n) {
const size_t old_size = size();
if (n < old_size) {
for (size_t i = n; i < old_size; ++i) {
auto hdr = Header();
auto hdr = ResolvedVectorHeader();
auto data = GetRuntime()->template ToAddress<::toolbelt::BufferOffset>(
hdr->data);
const ::toolbelt::BufferOffset offset = data[i];
Expand All @@ -1881,7 +1889,7 @@ class StringVectorField : public Field {
}
}
::toolbelt::PayloadBuffer::VectorResize<::toolbelt::BufferOffset>(
GetBufferAddr(), Header(), n);
GetBufferAddr(), ResolvedVectorHeader(), n);
}
void resize(size_t n, std::string_view value) {
const std::string value_copy(value);
Expand Down Expand Up @@ -1955,7 +1963,7 @@ class StringVectorField : public Field {
}

void Clear() {
auto hdr = Header();
auto hdr = ResolvedVectorHeader();
auto data =
GetRuntime()->template ToAddress<::toolbelt::BufferOffset>(hdr->data);
for (size_t i = 0; i < hdr->num_elements; ++i) {
Expand All @@ -1967,7 +1975,7 @@ class StringVectorField : public Field {
.Clear();
}
::toolbelt::PayloadBuffer::VectorClear<::toolbelt::BufferOffset>(
GetBufferAddr(), Header());
GetBufferAddr(), ResolvedVectorHeader());
}

void clear() { Clear(); } // STL compatibility.
Expand Down Expand Up @@ -2095,6 +2103,14 @@ class StringVectorField : public Field {
GetMessageBinaryStart() + relative_offset);
}

toolbelt::VectorHeader* ResolvedVectorHeader() const {
int32_t offset = FindFieldOffset(source_offset_);
if (offset < 0) {
offset = static_cast<int32_t>(relative_binary_offset_);
}
return Header(static_cast<uint32_t>(offset));
}

const std::shared_ptr<MessageRuntime>& GetRuntime() const {
return Message::GetRuntime(this, source_offset_);
}
Expand Down
Loading