Force static flatbuffer memory to be aligned
For buffers which required lots of alignment, we were allocating more
space than needed, and manually aligning inside that space. That
doesn't have a good enough contract to ensure that if the flatbuffer is
loaded back into RAM, it would still be aligned well enough for the
requirements.
Instead, we need to push the alignment requirement out to the allocator,
and then make sure we stay aligned inside the buffer. The std::vector<>
allocator isn't guarenteed to be aligned, so switch everything over to
the AlignedVectorAllocator instead which is aligned.
Change-Id: Ice2aa1316914472f2a3d55f470a4dc957e2caa3c
Signed-off-by: James Kuszmaul <james.kuszmaul@bluerivertech.com>
diff --git a/aos/events/logging/BUILD b/aos/events/logging/BUILD
index b58d6ad..2bb87b6 100644
--- a/aos/events/logging/BUILD
+++ b/aos/events/logging/BUILD
@@ -928,7 +928,7 @@
target_compatible_with = ["@platforms//os:linux"],
deps = [
":multinode_logger_test_lib",
- "//aos/flatbuffers:aligned_allocator",
+ "//aos/flatbuffers:base",
],
)
diff --git a/aos/events/logging/multinode_logger_test.cc b/aos/events/logging/multinode_logger_test.cc
index c252616..1e26c3c 100644
--- a/aos/events/logging/multinode_logger_test.cc
+++ b/aos/events/logging/multinode_logger_test.cc
@@ -8,7 +8,6 @@
#include "aos/events/message_counter.h"
#include "aos/events/ping_lib.h"
#include "aos/events/pong_lib.h"
-#include "aos/flatbuffers/aligned_allocator.h"
#include "aos/network/remote_message_generated.h"
#include "aos/network/timestamp_generated.h"
#include "aos/testing/tmpdir.h"
diff --git a/aos/flatbuffers/BUILD b/aos/flatbuffers/BUILD
index 1d6c602..0f25875 100644
--- a/aos/flatbuffers/BUILD
+++ b/aos/flatbuffers/BUILD
@@ -19,13 +19,21 @@
cc_library(
name = "base",
- srcs = ["base.cc"],
- hdrs = ["base.h"],
+ srcs = [
+ "base.cc",
+ ],
+ hdrs = [
+ "base.h",
+ ],
target_compatible_with = ["@platforms//os:linux"],
visibility = ["//visibility:public"],
deps = [
+ "//aos/containers:resizeable_buffer",
+ "//aos/ipc_lib:data_alignment",
"@com_github_google_flatbuffers//:flatbuffers",
"@com_github_google_glog//:glog",
+ "@com_google_absl//absl/base",
+ "@com_google_absl//absl/types:span",
],
)
@@ -33,7 +41,6 @@
name = "base_test",
srcs = ["base_test.cc"],
deps = [
- ":aligned_allocator",
":base",
"//aos/testing:googletest",
],
@@ -166,17 +173,3 @@
srcs = ["test_static.h"],
visibility = [":__subpackages__"],
)
-
-cc_library(
- name = "aligned_allocator",
- srcs = ["aligned_allocator.cc"],
- hdrs = ["aligned_allocator.h"],
- visibility = ["//visibility:public"],
- deps = [
- ":base",
- "//aos/containers:resizeable_buffer",
- "//aos/events:event_loop",
- "//aos/ipc_lib:data_alignment",
- "@com_github_google_glog//:glog",
- ],
-)
diff --git a/aos/flatbuffers/aligned_allocator.cc b/aos/flatbuffers/aligned_allocator.cc
deleted file mode 100644
index d2c47e5..0000000
--- a/aos/flatbuffers/aligned_allocator.cc
+++ /dev/null
@@ -1,96 +0,0 @@
-#include "aos/flatbuffers/aligned_allocator.h"
-
-namespace aos::fbs {
-
-AlignedVectorAllocator::~AlignedVectorAllocator() {
- CHECK(buffer_.empty())
- << ": Must deallocate before destroying the AlignedVectorAllocator.";
-}
-
-std::optional<std::span<uint8_t>> AlignedVectorAllocator::Allocate(
- size_t size, size_t /*alignment*/, fbs::SetZero set_zero) {
- CHECK(buffer_.empty()) << ": Must deallocate before calling Allocate().";
- buffer_.resize(((size + kAlignment - 1) / kAlignment) * kAlignment);
- allocated_size_ = size;
- if (set_zero == fbs::SetZero::kYes) {
- memset(buffer_.data(), 0, buffer_.size());
- }
-
- return std::span<uint8_t>{data(), allocated_size_};
-}
-
-std::optional<std::span<uint8_t>> AlignedVectorAllocator::InsertBytes(
- void *insertion_point, size_t bytes, size_t /*alignment*/,
- fbs::SetZero set_zero) {
- DCHECK_GE(reinterpret_cast<const uint8_t *>(insertion_point), data());
- DCHECK_LE(reinterpret_cast<const uint8_t *>(insertion_point),
- data() + allocated_size_);
- const size_t buffer_offset =
- reinterpret_cast<const uint8_t *>(insertion_point) - data();
- // TODO(austin): This has an extra memcpy in it that isn't strictly needed
- // when we resize. Remove it if performance is a concern.
- const size_t absolute_buffer_offset =
- reinterpret_cast<const uint8_t *>(insertion_point) - buffer_.data();
- const size_t previous_size = buffer_.size();
-
- buffer_.resize(((allocated_size_ + bytes + kAlignment - 1) / kAlignment) *
- kAlignment);
-
- // Now, we've got space both before and after the block of data. Move the
- // data after to the end, and the data before to the start.
-
- const size_t new_space_after = buffer_.size() - previous_size;
-
- // Move the rest of the data to be end aligned. If the buffer wasn't resized,
- // this will be a nop.
- memmove(buffer_.data() + absolute_buffer_offset + new_space_after,
- buffer_.data() + absolute_buffer_offset,
- previous_size - absolute_buffer_offset);
-
- // Now, move the data at the front to be aligned too.
- memmove(buffer_.data() + buffer_.size() - (allocated_size_ + bytes),
- buffer_.data() + previous_size - allocated_size_,
- allocated_size_ - (previous_size - absolute_buffer_offset));
-
- if (set_zero == fbs::SetZero::kYes) {
- memset(data() - bytes + buffer_offset, 0, bytes);
- }
- allocated_size_ += bytes;
-
- return std::span<uint8_t>{data(), allocated_size_};
-}
-
-std::span<uint8_t> AlignedVectorAllocator::RemoveBytes(
- std::span<uint8_t> remove_bytes) {
- const ssize_t removal_index = remove_bytes.data() - buffer_.data();
- const size_t old_start_index = buffer_.size() - allocated_size_;
- CHECK_LE(static_cast<ssize_t>(old_start_index), removal_index);
- CHECK_LE(removal_index, static_cast<ssize_t>(buffer_.size()));
- CHECK_LE(removal_index + remove_bytes.size(), buffer_.size());
- uint8_t *old_buffer_start = buffer_.data() + old_start_index;
- memmove(old_buffer_start + remove_bytes.size(), old_buffer_start,
- removal_index - old_start_index);
- allocated_size_ -= remove_bytes.size();
-
- return std::span<uint8_t>{data(), allocated_size_};
-}
-
-void AlignedVectorAllocator::Deallocate(std::span<uint8_t>) {
- if (!released_) {
- CHECK(!buffer_.empty())
- << ": Called Deallocate() without a prior allocation.";
- }
- released_ = false;
- buffer_.resize(0);
-}
-
-aos::SharedSpan AlignedVectorAllocator::Release() {
- absl::Span<uint8_t> span{data(), allocated_size_};
- std::shared_ptr<SharedSpanHolder> result = std::make_shared<SharedSpanHolder>(
- std::move(buffer_), absl::Span<const uint8_t>());
- result->span = span;
- released_ = true;
- return aos::SharedSpan(result, &(result->span));
-}
-
-} // namespace aos::fbs
diff --git a/aos/flatbuffers/aligned_allocator.h b/aos/flatbuffers/aligned_allocator.h
deleted file mode 100644
index a974818..0000000
--- a/aos/flatbuffers/aligned_allocator.h
+++ /dev/null
@@ -1,57 +0,0 @@
-#ifndef AOS_FLATBUFFERS_ALIGNED_ALLOCATOR_H_
-#define AOS_FLATBUFFERS_ALIGNED_ALLOCATOR_H_
-
-#include <memory>
-#include <optional>
-#include <span>
-
-#include "glog/logging.h"
-
-#include "aos/containers/resizeable_buffer.h"
-#include "aos/events/event_loop.h"
-#include "aos/flatbuffers/base.h"
-#include "aos/ipc_lib/data_alignment.h"
-
-namespace aos::fbs {
-
-// Allocator that uses an AllocatorResizeableBuffer to allow arbitrary-sized
-// allocations. Aligns the end of the buffer to an alignment of
-// kChannelDataAlignment.
-class AlignedVectorAllocator : public fbs::Allocator {
- public:
- static constexpr size_t kAlignment = aos::kChannelDataAlignment;
- AlignedVectorAllocator() {}
- ~AlignedVectorAllocator();
-
- std::optional<std::span<uint8_t>> Allocate(size_t size, size_t alignment,
- fbs::SetZero set_zero) override;
-
- std::optional<std::span<uint8_t>> InsertBytes(void *insertion_point,
- size_t bytes, size_t alignment,
- fbs::SetZero set_zero) override;
-
- std::span<uint8_t> RemoveBytes(std::span<uint8_t> remove_bytes) override;
-
- void Deallocate(std::span<uint8_t>) override;
-
- aos::SharedSpan Release();
-
- private:
- struct SharedSpanHolder {
- aos::AllocatorResizeableBuffer<
- aos::AlignedReallocator<kChannelDataAlignment>>
- buffer;
- absl::Span<const uint8_t> span;
- };
- uint8_t *data() { return buffer_.data() + buffer_.size() - allocated_size_; }
-
- aos::AllocatorResizeableBuffer<aos::AlignedReallocator<kChannelDataAlignment>>
- buffer_;
-
- size_t allocated_size_ = 0u;
- bool released_ = false;
-};
-
-} // namespace aos::fbs
-
-#endif // AOS_FLATBUFFERS_ALIGNED_ALLOCATOR_H_
diff --git a/aos/flatbuffers/base.cc b/aos/flatbuffers/base.cc
index 97b3b36..f22fce9 100644
--- a/aos/flatbuffers/base.cc
+++ b/aos/flatbuffers/base.cc
@@ -136,39 +136,8 @@
}
}
-std::optional<std::span<uint8_t>> VectorAllocator::Allocate(
- size_t size, size_t /*alignment*/, SetZero set_zero) {
- CHECK(buffer_.empty()) << ": Must deallocate before calling Allocate().";
- buffer_.resize(size);
- if (set_zero == SetZero::kYes) {
- memset(buffer_.data(), 0, buffer_.size());
- }
- return std::span<uint8_t>{buffer_.data(), buffer_.size()};
-}
-
-std::optional<std::span<uint8_t>> VectorAllocator::InsertBytes(
- void *insertion_point, size_t bytes, size_t /*alignment*/, SetZero) {
- const ssize_t insertion_index =
- reinterpret_cast<uint8_t *>(insertion_point) - buffer_.data();
- CHECK_LE(0, insertion_index);
- CHECK_LE(insertion_index, static_cast<ssize_t>(buffer_.size()));
- buffer_.insert(buffer_.begin() + insertion_index, bytes, 0);
- return std::span<uint8_t>{buffer_.data(), buffer_.size()};
-}
-
-std::span<uint8_t> VectorAllocator::RemoveBytes(
- std::span<uint8_t> remove_bytes) {
- const ssize_t removal_index = remove_bytes.data() - buffer_.data();
- CHECK_LE(0, removal_index);
- CHECK_LE(removal_index, static_cast<ssize_t>(buffer_.size()));
- CHECK_LE(removal_index + remove_bytes.size(), buffer_.size());
- buffer_.erase(buffer_.begin() + removal_index,
- buffer_.begin() + removal_index + remove_bytes.size());
- return {buffer_.data(), buffer_.size()};
-}
-
std::optional<std::span<uint8_t>> SpanAllocator::Allocate(size_t size,
- size_t /*alignment*/,
+ size_t alignment,
SetZero set_zero) {
CHECK(!allocated_);
if (size > buffer_.size()) {
@@ -179,6 +148,10 @@
}
allocated_size_ = size;
allocated_ = true;
+ CHECK_GT(alignment, 0u);
+ CHECK_EQ(buffer_.size() % alignment, 0u)
+ << ": Buffer isn't a multiple of alignment " << alignment << " long, is "
+ << buffer_.size() << " long";
return internal::GetSubSpan(buffer_, buffer_.size() - size);
}
@@ -223,6 +196,97 @@
allocated_ = false;
}
+AlignedVectorAllocator::~AlignedVectorAllocator() {
+ CHECK(buffer_.empty())
+ << ": Must deallocate before destroying the AlignedVectorAllocator.";
+}
+
+std::optional<std::span<uint8_t>> AlignedVectorAllocator::Allocate(
+ size_t size, size_t /*alignment*/, fbs::SetZero set_zero) {
+ CHECK(buffer_.empty()) << ": Must deallocate before calling Allocate().";
+ buffer_.resize(((size + kAlignment - 1) / kAlignment) * kAlignment);
+ allocated_size_ = size;
+ if (set_zero == fbs::SetZero::kYes) {
+ memset(buffer_.data(), 0, buffer_.size());
+ }
+
+ return std::span<uint8_t>{data(), allocated_size_};
+}
+
+std::optional<std::span<uint8_t>> AlignedVectorAllocator::InsertBytes(
+ void *insertion_point, size_t bytes, size_t /*alignment*/,
+ fbs::SetZero set_zero) {
+ DCHECK_GE(reinterpret_cast<const uint8_t *>(insertion_point), data());
+ DCHECK_LE(reinterpret_cast<const uint8_t *>(insertion_point),
+ data() + allocated_size_);
+ const size_t buffer_offset =
+ reinterpret_cast<const uint8_t *>(insertion_point) - data();
+ // TODO(austin): This has an extra memcpy in it that isn't strictly needed
+ // when we resize. Remove it if performance is a concern.
+ const size_t absolute_buffer_offset =
+ reinterpret_cast<const uint8_t *>(insertion_point) - buffer_.data();
+ const size_t previous_size = buffer_.size();
+
+ buffer_.resize(((allocated_size_ + bytes + kAlignment - 1) / kAlignment) *
+ kAlignment);
+
+ // Now, we've got space both before and after the block of data. Move the
+ // data after to the end, and the data before to the start.
+
+ const size_t new_space_after = buffer_.size() - previous_size;
+
+ // Move the rest of the data to be end aligned. If the buffer wasn't resized,
+ // this will be a nop.
+ memmove(buffer_.data() + absolute_buffer_offset + new_space_after,
+ buffer_.data() + absolute_buffer_offset,
+ previous_size - absolute_buffer_offset);
+
+ // Now, move the data at the front to be aligned too.
+ memmove(buffer_.data() + buffer_.size() - (allocated_size_ + bytes),
+ buffer_.data() + previous_size - allocated_size_,
+ allocated_size_ - (previous_size - absolute_buffer_offset));
+
+ if (set_zero == fbs::SetZero::kYes) {
+ memset(data() - bytes + buffer_offset, 0, bytes);
+ }
+ allocated_size_ += bytes;
+
+ return std::span<uint8_t>{data(), allocated_size_};
+}
+
+std::span<uint8_t> AlignedVectorAllocator::RemoveBytes(
+ std::span<uint8_t> remove_bytes) {
+ const ssize_t removal_index = remove_bytes.data() - buffer_.data();
+ const size_t old_start_index = buffer_.size() - allocated_size_;
+ CHECK_LE(static_cast<ssize_t>(old_start_index), removal_index);
+ CHECK_LE(removal_index, static_cast<ssize_t>(buffer_.size()));
+ CHECK_LE(removal_index + remove_bytes.size(), buffer_.size());
+ uint8_t *old_buffer_start = buffer_.data() + old_start_index;
+ memmove(old_buffer_start + remove_bytes.size(), old_buffer_start,
+ removal_index - old_start_index);
+ allocated_size_ -= remove_bytes.size();
+
+ return std::span<uint8_t>{data(), allocated_size_};
+}
+
+void AlignedVectorAllocator::Deallocate(std::span<uint8_t>) {
+ if (!released_) {
+ CHECK(!buffer_.empty())
+ << ": Called Deallocate() without a prior allocation.";
+ }
+ released_ = false;
+ buffer_.resize(0);
+}
+
+aos::SharedSpan AlignedVectorAllocator::Release() {
+ absl::Span<uint8_t> span{data(), allocated_size_};
+ std::shared_ptr<SharedSpanHolder> result = std::make_shared<SharedSpanHolder>(
+ std::move(buffer_), absl::Span<const uint8_t>());
+ result->span = span;
+ released_ = true;
+ return aos::SharedSpan(result, &(result->span));
+}
+
namespace internal {
std::ostream &DebugBytes(std::span<const uint8_t> span, std::ostream &os) {
constexpr size_t kRowSize = 8u;
diff --git a/aos/flatbuffers/base.h b/aos/flatbuffers/base.h
index a92cc91..b3c3ef0 100644
--- a/aos/flatbuffers/base.h
+++ b/aos/flatbuffers/base.h
@@ -12,10 +12,17 @@
#include <utility>
#include <vector>
+#include "absl/types/span.h"
#include "flatbuffers/base.h"
#include "glog/logging.h"
-namespace aos::fbs {
+#include "aos/containers/resizeable_buffer.h"
+#include "aos/ipc_lib/data_alignment.h"
+
+namespace aos {
+using SharedSpan = std::shared_ptr<const absl::Span<const uint8_t>>;
+
+namespace fbs {
using ::flatbuffers::soffset_t;
using ::flatbuffers::uoffset_t;
@@ -176,8 +183,9 @@
class Allocator {
public:
virtual ~Allocator() {}
- // Allocates memory of the requested size and alignment. alignment is not
+ // Allocates memory of the requested size and alignment. alignment is
// guaranteed.
+ //
// On failure to allocate the requested size, returns nullopt;
// Never returns a partial span.
// The span will be initialized to zero upon request.
@@ -185,16 +193,19 @@
// Deallocate() has been called. In order to adjust the size of the buffer,
// call InsertBytes() and RemoveBytes().
[[nodiscard]] virtual std::optional<std::span<uint8_t>> Allocate(
- size_t size, size_t alignment_hint, SetZero set_zero) = 0;
+ size_t size, size_t alignment, SetZero set_zero) = 0;
// Identical to Allocate(), but dies on failure.
- [[nodiscard]] std::span<uint8_t> AllocateOrDie(size_t size,
- size_t alignment_hint,
+ [[nodiscard]] std::span<uint8_t> AllocateOrDie(size_t size, size_t alignment,
SetZero set_zero) {
std::optional<std::span<uint8_t>> span =
- Allocate(size, alignment_hint, set_zero);
+ Allocate(size, alignment, set_zero);
CHECK(span.has_value()) << ": Failed to allocate " << size << " bytes.";
CHECK_EQ(size, span.value().size())
<< ": Failed to allocate " << size << " bytes.";
+ CHECK_EQ(reinterpret_cast<size_t>(span.value().data()) % alignment, 0u)
+ << "Failed to allocate data of length " << size << " with alignment "
+ << alignment;
+
return span.value();
}
// Increases the size of the buffer by inserting bytes bytes immediately
@@ -221,33 +232,6 @@
virtual void Deallocate(std::span<uint8_t> buffer) = 0;
};
-// Allocator that uses an std::vector to allow arbitrary-sized allocations.
-// Does not provide any alignment guarantees.
-class VectorAllocator : public Allocator {
- public:
- VectorAllocator() {}
- ~VectorAllocator() {
- CHECK(buffer_.empty())
- << ": Must deallocate before destroying the VectorAllocator.";
- }
- std::optional<std::span<uint8_t>> Allocate(size_t size, size_t /*alignment*/,
- SetZero set_zero) override;
- std::optional<std::span<uint8_t>> InsertBytes(void *insertion_point,
- size_t bytes,
- size_t /*alignment*/,
- SetZero /*set_zero*/) override;
- std::span<uint8_t> RemoveBytes(std::span<uint8_t> remove_bytes) override;
-
- void Deallocate(std::span<uint8_t>) override {
- CHECK(!buffer_.empty())
- << ": Called Deallocate() without a prior allocation.";
- buffer_.resize(0);
- }
-
- private:
- std::vector<uint8_t> buffer_;
-};
-
// Allocator that allocates all of its memory within a provided span. To match
// the behavior of the FlatBufferBuilder, it will start its allocations at the
// end of the provided span.
@@ -280,17 +264,52 @@
size_t allocated_size_ = 0;
};
+// Allocator that uses an AllocatorResizeableBuffer to allow arbitrary-sized
+// allocations. Aligns the end of the buffer to an alignment of
+// kChannelDataAlignment.
+class AlignedVectorAllocator : public fbs::Allocator {
+ public:
+ static constexpr size_t kAlignment = aos::kChannelDataAlignment;
+ AlignedVectorAllocator() {}
+ ~AlignedVectorAllocator();
+
+ std::optional<std::span<uint8_t>> Allocate(size_t size, size_t alignment,
+ fbs::SetZero set_zero) override;
+
+ std::optional<std::span<uint8_t>> InsertBytes(void *insertion_point,
+ size_t bytes, size_t alignment,
+ fbs::SetZero set_zero) override;
+
+ std::span<uint8_t> RemoveBytes(std::span<uint8_t> remove_bytes) override;
+
+ void Deallocate(std::span<uint8_t>) override;
+
+ aos::SharedSpan Release();
+
+ private:
+ struct SharedSpanHolder {
+ aos::AllocatorResizeableBuffer<aos::AlignedReallocator<kAlignment>> buffer;
+ absl::Span<const uint8_t> span;
+ };
+ uint8_t *data() { return buffer_.data() + buffer_.size() - allocated_size_; }
+
+ aos::AllocatorResizeableBuffer<aos::AlignedReallocator<kAlignment>> buffer_;
+
+ size_t allocated_size_ = 0u;
+ bool released_ = false;
+};
+
// Allocates and owns a fixed-size memory buffer on the stack.
//
// This provides a convenient Allocator for use with the aos::fbs::Builder
// in realtime code instead of trying to use the VectorAllocator.
-template <std::size_t N>
+template <std::size_t N, std::size_t alignment = 64>
class FixedStackAllocator : public SpanAllocator {
public:
FixedStackAllocator() : SpanAllocator({buffer_, sizeof(buffer_)}) {}
private:
- uint8_t buffer_[N];
+ alignas(alignment) uint8_t buffer_[N];
};
namespace internal {
@@ -324,5 +343,7 @@
T t;
};
} // namespace internal
-} // namespace aos::fbs
+} // namespace fbs
+} // namespace aos
+
#endif // AOS_FLATBUFFERS_BASE_H_
diff --git a/aos/flatbuffers/base_test.cc b/aos/flatbuffers/base_test.cc
index 87d89fa..43c9c5b 100644
--- a/aos/flatbuffers/base_test.cc
+++ b/aos/flatbuffers/base_test.cc
@@ -6,8 +6,6 @@
#include "gtest/gtest.h"
-#include "aos/flatbuffers/aligned_allocator.h"
-
namespace aos::fbs::testing {
// Tests that PaddedSize() behaves as expected.
TEST(BaseTest, PaddedSize) {
@@ -23,19 +21,18 @@
class AllocatorTest : public ::testing::Test {
protected:
AllocatorTest() : allocator_(std::make_unique<T>()) {}
- std::vector<uint8_t> buffer_;
+ alignas(64) std::array<uint8_t, kDefaultSize> buffer_;
// unique_ptr so that we can destroy the allocator at will.
std::unique_ptr<T> allocator_;
};
template <>
AllocatorTest<SpanAllocator>::AllocatorTest()
- : buffer_(kDefaultSize),
- allocator_(std::make_unique<SpanAllocator>(
+ : allocator_(std::make_unique<SpanAllocator>(
std::span<uint8_t>{buffer_.data(), buffer_.size()})) {}
-using AllocatorTypes =
- ::testing::Types<SpanAllocator, VectorAllocator, AlignedVectorAllocator>;
+using AllocatorTypes = ::testing::Types<SpanAllocator, AlignedVectorAllocator,
+ FixedStackAllocator<kDefaultSize>>;
TYPED_TEST_SUITE(AllocatorTest, AllocatorTypes);
// Tests that we can create and not use a VectorAllocator.
@@ -80,6 +77,17 @@
this->allocator_->Deallocate(span);
}
+// Tests that all allocators return data aligned to the requested alignment.
+TYPED_TEST(AllocatorTest, Alignment) {
+ for (size_t alignment : {4, 8, 16, 32, 64}) {
+ std::span<uint8_t> span =
+ this->allocator_->Allocate(kDefaultSize, alignment, SetZero::kYes)
+ .value();
+ EXPECT_EQ(reinterpret_cast<size_t>(span.data()) % alignment, 0);
+ this->allocator_->Deallocate(span);
+ }
+}
+
// Tests that we can remove bytes from an arbitrary spot in the buffer.
TYPED_TEST(AllocatorTest, RemoveBytes) {
// Deletion doesn't require resizing, so we don't need to worry about it being
@@ -142,7 +150,7 @@
std::vector<uint8_t> buffer(kDefaultSize);
SpanAllocator allocator({buffer.data(), buffer.size()});
std::span<uint8_t> span =
- allocator.Allocate(kDefaultSize, 0, SetZero::kYes).value();
+ allocator.Allocate(kDefaultSize, 1, SetZero::kYes).value();
EXPECT_EQ(kDefaultSize, span.size());
EXPECT_FALSE(
allocator.InsertBytes(span.data(), 1u, 0, SetZero::kYes).has_value());
@@ -209,7 +217,7 @@
: object_(allocator_.Allocate(kInitialSize, 4, SetZero::kYes).value(),
&allocator_) {}
~ResizeableObjectTest() { allocator_.Deallocate(object_.buffer()); }
- VectorAllocator allocator_;
+ AlignedVectorAllocator allocator_;
TestResizeableObject object_;
};
diff --git a/aos/flatbuffers/builder.h b/aos/flatbuffers/builder.h
index 36225c0..b4aefbe 100644
--- a/aos/flatbuffers/builder.h
+++ b/aos/flatbuffers/builder.h
@@ -18,7 +18,8 @@
template <typename T>
class Builder final : public ResizeableObject {
public:
- static constexpr size_t kBufferSize = T::kUnalignedBufferSize;
+ static constexpr size_t kBufferSize = T::kRootSize;
+ static constexpr size_t kAlign = T::kAlign;
// Note on memory initialization: We zero-initialize all the memory that we
// create at the start. While this can be overkill, it is simpler to manage
// the alternatives, and we don't currently have a clear performance need for
@@ -48,7 +49,7 @@
SetPrefix();
}
Builder(std::unique_ptr<Allocator> allocator =
- std::make_unique<VectorAllocator>())
+ std::make_unique<AlignedVectorAllocator>())
: ResizeableObject(
allocator->AllocateOrDie(kBufferSize, T::kAlign, SetZero::kYes),
std::move(allocator)),
diff --git a/aos/flatbuffers/static_flatbuffers_test.cc b/aos/flatbuffers/static_flatbuffers_test.cc
index 66938f1..0929d17 100644
--- a/aos/flatbuffers/static_flatbuffers_test.cc
+++ b/aos/flatbuffers/static_flatbuffers_test.cc
@@ -108,7 +108,7 @@
// Test that compiles the same code that is used by an example in
// //aos/documentation/aos/docs/flatbuffers.md.
TEST_F(StaticFlatbuffersTest, DocumentationExample) {
- aos::fbs::VectorAllocator allocator;
+ aos::fbs::AlignedVectorAllocator allocator;
Builder<TestTableStatic> builder(&allocator);
TestTableStatic *object = builder.get();
object->set_scalar(123);
@@ -176,7 +176,7 @@
aos::FlatbufferDetachedBuffer<TestTable> fbb_finished = fbb.Release();
// Using the static flatbuffer API.
- aos::fbs::VectorAllocator allocator;
+ aos::fbs::AlignedVectorAllocator allocator;
Builder<TestTableStatic> static_builder(&allocator);
PopulateStatic(CHECK_NOTNULL(static_builder.get()->add_subtable()));
@@ -218,7 +218,7 @@
// it stays valid at all points.
TEST_F(StaticFlatbuffersTest, ManuallyConstructFlatbuffer) {
{
- aos::fbs::VectorAllocator allocator;
+ aos::fbs::AlignedVectorAllocator allocator;
Builder<SubTableStatic> builder(&allocator);
SubTableStatic *object = builder.get();
if (!builder.AsFlatbufferSpan().Verify()) {
@@ -241,9 +241,7 @@
TestMemory(builder.buffer());
}
{
- // aos::FixedAllocator
- // allocator(TestTableStatic::kUnalignedBufferSize);
- aos::fbs::VectorAllocator allocator;
+ aos::fbs::AlignedVectorAllocator allocator;
Builder<TestTableStatic> builder(&allocator);
TestTableStatic *object = builder.get();
const aos::fbs::testing::TestTable &fbs = object->AsFlatbuffer();
@@ -673,7 +671,7 @@
// Tests that field clearing (and subsequent resetting) works properly.
TEST_F(StaticFlatbuffersTest, ClearFields) {
- aos::fbs::VectorAllocator allocator;
+ aos::fbs::AlignedVectorAllocator allocator;
Builder<TestTableStatic> builder(&allocator);
TestTableStatic *object = builder.get();
// For each field, we will confirm the following:
@@ -859,7 +857,7 @@
// Confirm that we can use the SpanAllocator with a span that provides exactly
// the required buffer size.
TEST_F(StaticFlatbuffersTest, ExactSizeSpanAllocator) {
- uint8_t buffer[Builder<TestTableStatic>::kBufferSize];
+ alignas(64) uint8_t buffer[Builder<TestTableStatic>::kBufferSize];
aos::fbs::SpanAllocator allocator({buffer, sizeof(buffer)});
Builder<TestTableStatic> builder(&allocator);
TestTableStatic *object = builder.get();
@@ -922,7 +920,7 @@
// Verify that if we create a span with extra headroom that that lets us
// dynamically alter the size of vectors in the flatbuffers.
TEST_F(StaticFlatbuffersTest, ExtraLargeSpanAllocator) {
- uint8_t buffer[Builder<TestTableStatic>::kBufferSize + 10000];
+ alignas(64) uint8_t buffer[Builder<TestTableStatic>::kBufferSize + 200 * 64];
aos::fbs::SpanAllocator allocator({buffer, sizeof(buffer)});
Builder<TestTableStatic> builder(&allocator);
TestTableStatic *object = builder.get();
@@ -951,7 +949,7 @@
// Tests that the iterators on the Vector type work.
TEST_F(StaticFlatbuffersTest, IteratorTest) {
- Builder<TestTableStatic> builder(std::make_unique<VectorAllocator>());
+ Builder<TestTableStatic> builder(std::make_unique<AlignedVectorAllocator>());
{
auto vector = builder->add_unspecified_length_vector();
ASSERT_TRUE(vector->reserve(9000));
@@ -1019,7 +1017,8 @@
// Confirm that we can use the FixedStackAllocator
TEST_F(StaticFlatbuffersTest, FixedStackAllocator) {
- aos::fbs::FixedStackAllocator<Builder<TestTableStatic>::kBufferSize>
+ aos::fbs::FixedStackAllocator<Builder<TestTableStatic>::kBufferSize,
+ Builder<TestTableStatic>::kAlign>
allocator;
Builder<TestTableStatic> builder(&allocator);
TestTableStatic *object = builder.get();
@@ -1078,7 +1077,7 @@
object_t.vector_of_strings.push_back("971");
object_t.vector_of_structs.push_back({1, 2});
object_t.subtable = std::make_unique<SubTableT>();
- aos::fbs::VectorAllocator allocator;
+ aos::fbs::AlignedVectorAllocator allocator;
Builder<TestTableStatic> builder(&allocator);
ASSERT_TRUE(builder->FromFlatbuffer(object_t));
ASSERT_TRUE(builder.AsFlatbufferSpan().Verify());
@@ -1123,7 +1122,7 @@
// Tests that we can use the move constructor on a Builder.
TEST_F(StaticFlatbuffersTest, BuilderMoveConstructor) {
- uint8_t buffer[Builder<TestTableStatic>::kBufferSize];
+ alignas(64) uint8_t buffer[Builder<TestTableStatic>::kBufferSize];
aos::fbs::SpanAllocator allocator({buffer, sizeof(buffer)});
Builder<TestTableStatic> builder_from(&allocator);
Builder<TestTableStatic> builder(std::move(builder_from));
diff --git a/aos/json_to_flatbuffer.h b/aos/json_to_flatbuffer.h
index deafaa7..f83bab9 100644
--- a/aos/json_to_flatbuffer.h
+++ b/aos/json_to_flatbuffer.h
@@ -43,7 +43,7 @@
inline fbs::Builder<T> JsonToStaticFlatbuffer(const std::string_view data) {
const aos::FlatbufferDetachedBuffer<typename T::Flatbuffer> fbs =
JsonToFlatbuffer<typename T::Flatbuffer>(data);
- fbs::Builder<T> builder(std::make_unique<aos::fbs::VectorAllocator>());
+ fbs::Builder<T> builder(std::make_unique<aos::fbs::AlignedVectorAllocator>());
CHECK(builder.get()->FromFlatbuffer(&fbs.message()));
return builder;
}
diff --git a/documentation/aos/docs/flatbuffers.md b/documentation/aos/docs/flatbuffers.md
index 9c82ee3..ea8c865 100644
--- a/documentation/aos/docs/flatbuffers.md
+++ b/documentation/aos/docs/flatbuffers.md
@@ -55,7 +55,7 @@
templated `aos::fbs::Builder` object is provided which can
take an allocator and then provide the relevant table class to the user.
* We provide an `Allocator` class and various implementations (e.g., a
- `VectorAllocator` backed by an `std::vector`) for managing the memory into
+ `AlignedVectorAllocator` backed by an `std::vector`) for managing the memory into
which the `Builder` will serialize the flatbuffer.
* A new `MakeStaticBuilder` method is provided on the `aos::Sender` class which
constructs an `aos::fbs::Builder` to allow you to construct a message to be
@@ -92,7 +92,7 @@
In order to handle alignment correctly in our `Builder` and `Allocator` classes,
we end up forcing the `Builder` to be able to accept semi-arbitrarily aligned
buffers in order to ease the `Allocator` implementation (e.g., the
-`VectorAllocator` uses a `std::vector` internally which does not necessarily
+`AlignedVectorAllocator` uses a `std::vector` internally which does not necessarily
align its memory). The `Builder` then adds padding as needed and passes an
appropriately aligned buffer down to the `Table` class.
@@ -299,7 +299,7 @@
aos::FlatbufferDetachedBuffer<TestTable> fbb_finished = fbb.Release();
// Using the static flatbuffer API.
- aos::fbs::VectorAllocator allocator;
+ aos::fbs::AlignedVectorAllocator allocator;
Builder<TestTableStatic> static_builder(&allocator);
PopulateStatic(CHECK_NOTNULL(static_builder.get()->add_subtable()));