diff --git a/folly/container/tape.h b/folly/container/tape.h index e46720e64d0..317a1193ae0 100644 --- a/folly/container/tape.h +++ b/folly/container/tape.h @@ -64,7 +64,8 @@ namespace folly { * `record_builder` interface. * * Existing `records` can be accessed by index. - * Existing `records` cannot be mutated. + * Existing `records` cannot be mutated, except for the last record (see record + * builder). * * ## tape * @@ -276,8 +277,13 @@ class tape { // record builder (constructing last record) ------- - class scoped_record_builder; - [[nodiscard]] scoped_record_builder record_builder(); + class record_builder; + + // get a record builder. + // new_record_builder starts a builder for a new record. + // last_record_builder allows you to append/mutate the last record. + [[nodiscard]] record_builder new_record_builder(); + [[nodiscard]] record_builder last_record_builder(); // insert one record ---------- @@ -328,6 +334,7 @@ class tape { // erase ------- void pop_back() noexcept { + assert(!empty()); data_.resize(data_.size() - back().size()); markers_.pop_back(); } @@ -369,13 +376,15 @@ class tape { // Provides a way to construct a last record similar // to how you would `std::vector`. +// Typical workflow is you `push_back` a bunch of individual elements and then +// `commit()`. template -class tape::scoped_record_builder { +class tape::record_builder { public: - scoped_record_builder(const scoped_record_builder&) = delete; - scoped_record_builder(scoped_record_builder&&) = delete; - scoped_record_builder& operator=(const scoped_record_builder&) = delete; - scoped_record_builder& operator=(scoped_record_builder&&) = delete; + record_builder(const record_builder&) = delete; + record_builder(record_builder&&) = delete; + record_builder& operator=(const record_builder&) = delete; + record_builder& operator=(record_builder&&) = delete; using iterator = typename container_type::iterator; using const_iterator = typename container_type::const_iterator; @@ -386,6 +395,23 @@ class tape::scoped_record_builder { using difference_type = typename std::iterator_traits::difference_type; + // mutators --- + + void push_back(scalar_value_type x) { self_->data_.push_back(std::move(x)); } + + template + reference emplace_back(Args&&... args) { + self_->data_.emplace_back(std::forward(args)...); + // cannot rely on the container doing the right thing here. + return self_->data_.back(); + } + + // constructed record is added to the tape. + void commit() { self_->markers_.push_back(self_->data_.size()); } + + // discards elements of the constructed record. (automatic on destruction) + void abort() { self_->data_.resize(self_->markers_.back()); } + // iterators ----- [[nodiscard]] iterator begin() noexcept { @@ -443,33 +469,26 @@ class tape::scoped_record_builder { [[nodiscard]] reference back() { return self_->data_.back(); } [[nodiscard]] const_reference back() const { return self_->data_.back(); } - // mutators --- - - void push_back(scalar_value_type x) { self_->data_.push_back(std::move(x)); } - - template - reference emplace_back(Args&&... args) { - self_->data_.emplace_back(std::forward(args)...); - // cannot rely on the container doing the right thing here. - return self_->data_.back(); - } - - ~scoped_record_builder() noexcept { - // we assume that allocations never fail - self_->markers_.push_back(self_->data_.size()); - } + ~record_builder() noexcept { abort(); } private: friend class tape; - explicit scoped_record_builder(tape& self) : self_(&self) {} + explicit record_builder(tape& self) : self_(&self) {} tape* self_; }; template -auto tape::record_builder() -> scoped_record_builder { - return scoped_record_builder{*this}; +auto tape::new_record_builder() -> record_builder { + return record_builder{*this}; +} + +template +auto tape::last_record_builder() -> record_builder { + assert(!empty()); + markers_.pop_back(); + return new_record_builder(); } // tape methods ----- diff --git a/folly/container/test/tape_bench.cpp b/folly/container/test/tape_bench.cpp index 2d135162b59..256c52f181d 100644 --- a/folly/container/test/tape_bench.cpp +++ b/folly/container/test/tape_bench.cpp @@ -109,11 +109,12 @@ void pushBackByOne( template void pushBackByOne( folly::tape>& cont, const std::vector>& in) { + auto builder = cont.new_record_builder(); for (const auto& v : in) { - auto builder = cont.record_builder(); for (const auto& x : v) { builder.push_back(x); } + builder.commit(); } } diff --git a/folly/container/test/tape_test.cpp b/folly/container/test/tape_test.cpp index 0d7d1bf7c5a..e0abc7a0928 100644 --- a/folly/container/test/tape_test.cpp +++ b/folly/container/test/tape_test.cpp @@ -248,7 +248,7 @@ TEST(Tape, RecordBuilder) { folly::string_tape st; { - auto builder = st.record_builder(); + auto builder = st.new_record_builder(); ASSERT_TRUE(builder.empty()); ASSERT_EQ(0U, builder.size()); builder.push_back('a'); @@ -262,27 +262,86 @@ TEST(Tape, RecordBuilder) { ASSERT_EQ(std::string_view(builder.begin(), builder.end()), "ab"); ASSERT_EQ(std::string_view(builder.cbegin(), builder.cend()), "ab"); *builder.begin() = 'c'; + builder.commit(); } { - auto builder = st.record_builder(); + auto builder = st.new_record_builder(); ASSERT_EQ(builder.emplace_back('d'), 'd'); builder[0] = 'e'; ASSERT_EQ(std::as_const(builder)[0], 'e'); + builder.commit(); } { - auto builder = st.record_builder(); + auto builder = st.new_record_builder(); builder.emplace_back(); // test emplace with default constructor builder.at(0) = 'b'; ASSERT_EQ(std::as_const(builder).at(0), 'b'); builder[0] = 'a'; ASSERT_EQ(std::as_const(builder)[0], 'a'); + builder.commit(); } { - auto builder = st.record_builder(); + auto builder = st.new_record_builder(); constexpr std::string_view s = "abc"; std::copy(s.begin(), s.end(), builder.back_inserter()); + builder.commit(); + } + { + auto builder = st.new_record_builder(); + builder.push_back('0'); + builder.push_back('1'); + // Default abort - not committed + } + { + // calling abort + auto builder = st.new_record_builder(); + ASSERT_EQ(0U, builder.size()); + builder.push_back('a'); + builder.push_back('b'); + ASSERT_EQ(2U, builder.size()); + builder.abort(); + ASSERT_EQ(0U, builder.size()); + builder.push_back('a'); + builder.push_back('b'); + builder.commit(); + } + { + // amending record + ASSERT_EQ(st.back(), "ab"); + auto builder = st.last_record_builder(); + builder.push_back('c'); + builder[0] = '0'; + builder.commit(); + ASSERT_EQ(st.back(), "0bc"); + } + { + // not committing last record + ASSERT_EQ(st.size(), 5); + + { + auto builder = st.new_record_builder(); + builder.push_back('a'); + builder.push_back('b'); + builder.commit(); + } + + ASSERT_EQ(st.size(), 6); + (void)st.last_record_builder(); // destructor will auto abort. + + ASSERT_EQ(st.size(), 5); } + ASSERT_EQ("cb", st[0]); + ASSERT_EQ("e", st[1]); + ASSERT_EQ("a", st[2]); + ASSERT_EQ("abc", st[3]); + ASSERT_EQ("0bc", st[4]); + ASSERT_EQ(10U, st.size_flat()); + ASSERT_EQ(5U, st.size()); + + // checking that pop back is still ok. + + st.pop_back(); ASSERT_EQ("cb", st[0]); ASSERT_EQ("e", st[1]); ASSERT_EQ("a", st[2]); @@ -515,18 +574,15 @@ TEST(Tape, Iteration) { TEST(Tape, TapeOfTapes) { folly::tape strings; - { - auto builder = strings.record_builder(); - builder.push_back("ab"); - builder.push_back("abc"); - } + auto builder = strings.new_record_builder(); + builder.push_back("ab"); + builder.push_back("abc"); + builder.commit(); - { - auto builder = strings.record_builder(); - builder.push_back("bc"); - builder.push_back("bcd"); - builder.push_back("bcde"); - } + builder.push_back("bc"); + builder.push_back("bcd"); + builder.push_back("bcde"); + builder.commit(); ASSERT_EQ(strings.size(), 2); ASSERT_THAT(strings[0], testing::ElementsAre("ab", "abc"));