From 07ebd1fa1fcd097e42181d569d3222792ab9e934 Mon Sep 17 00:00:00 2001 From: Alex Seaton Date: Wed, 7 Aug 2024 22:06:22 +0100 Subject: [PATCH] Get rid of Segment& segment() --- cpp/arcticdb/async/async_store.cpp | 3 +-- cpp/arcticdb/async/tasks.cpp | 10 +++++----- cpp/arcticdb/async/tasks.hpp | 13 +++++++++---- cpp/arcticdb/codec/codec.cpp | 3 +-- cpp/arcticdb/codec/codec.hpp | 2 +- cpp/arcticdb/codec/python_bindings.cpp | 2 +- cpp/arcticdb/codec/test/test_codec.cpp | 4 ++-- cpp/arcticdb/pipeline/read_frame.cpp | 11 ++++------- cpp/arcticdb/pipeline/read_frame.hpp | 4 ++-- cpp/arcticdb/storage/azure/azure_client_wrapper.hpp | 2 +- cpp/arcticdb/storage/azure/azure_mock_client.cpp | 2 +- cpp/arcticdb/storage/azure/azure_mock_client.hpp | 2 +- cpp/arcticdb/storage/azure/azure_real_client.cpp | 2 +- cpp/arcticdb/storage/azure/azure_real_client.hpp | 2 +- cpp/arcticdb/storage/azure/azure_storage.cpp | 3 +-- cpp/arcticdb/storage/file/mapped_file_storage.cpp | 8 ++++---- cpp/arcticdb/storage/file/mapped_file_storage.hpp | 2 +- cpp/arcticdb/storage/key_segment_pair.hpp | 12 +++++++++--- cpp/arcticdb/storage/library.hpp | 2 +- cpp/arcticdb/storage/lmdb/lmdb_client_wrapper.hpp | 2 +- cpp/arcticdb/storage/lmdb/lmdb_mock_client.cpp | 4 ++-- cpp/arcticdb/storage/lmdb/lmdb_mock_client.hpp | 2 +- cpp/arcticdb/storage/lmdb/lmdb_real_client.cpp | 2 +- cpp/arcticdb/storage/lmdb/lmdb_real_client.hpp | 2 +- cpp/arcticdb/storage/lmdb/lmdb_storage.cpp | 3 +-- cpp/arcticdb/storage/memory/memory_storage.cpp | 12 ++++++------ cpp/arcticdb/storage/mongo/mongo_client.cpp | 5 ++--- cpp/arcticdb/storage/mongo/mongo_mock_client.cpp | 4 ++-- cpp/arcticdb/storage/mongo/mongo_storage.cpp | 2 +- cpp/arcticdb/storage/s3/detail-inl.hpp | 6 ++---- cpp/arcticdb/storage/s3/s3_client_wrapper.hpp | 2 +- cpp/arcticdb/storage/s3/s3_mock_client.cpp | 4 ++-- cpp/arcticdb/storage/s3/s3_mock_client.hpp | 2 +- cpp/arcticdb/storage/s3/s3_real_client.cpp | 2 +- cpp/arcticdb/storage/s3/s3_real_client.hpp | 2 +- cpp/arcticdb/storage/storage.hpp | 2 +- cpp/arcticdb/toolbox/library_tool.cpp | 8 ++++---- 37 files changed, 78 insertions(+), 77 deletions(-) diff --git a/cpp/arcticdb/async/async_store.cpp b/cpp/arcticdb/async/async_store.cpp index ce49757f4f..0131be65eb 100644 --- a/cpp/arcticdb/async/async_store.cpp +++ b/cpp/arcticdb/async/async_store.cpp @@ -20,8 +20,7 @@ std::pair> lookup_match_in_dedup_map( ARCTICDB_DEBUG(log::version(), "No existing key with same contents: writing new object {}", key_seg.atom_key()); - return std::make_pair(std::move(key_seg.atom_key()), std::make_optional(std::move(key_seg.segment()))); - + return std::make_pair(key_seg.atom_key(), std::make_optional(std::move(*key_seg.release_segment()))); } else { ARCTICDB_DEBUG(log::version(), "Found existing key with same contents: using existing object {}", diff --git a/cpp/arcticdb/async/tasks.cpp b/cpp/arcticdb/async/tasks.cpp index b209f8528e..cdb1d5d0aa 100644 --- a/cpp/arcticdb/async/tasks.cpp +++ b/cpp/arcticdb/async/tasks.cpp @@ -37,17 +37,17 @@ namespace arcticdb::async { pipelines::SegmentAndSlice DecodeSliceTask::decode_into_slice(storage::KeySegmentPair&& key_segment_pair) { auto key = key_segment_pair.atom_key(); - auto& seg = key_segment_pair.segment(); + auto seg = key_segment_pair.segment_ptr(); ARCTICDB_DEBUG(log::storage(), "ReadAndDecodeAtomTask decoding segment of size {} with key {}", - seg.size(), + seg->size(), key); - auto &hdr = seg.header(); - const auto& desc = seg.descriptor(); + auto &hdr = seg->header(); + const auto& desc = seg->descriptor(); auto descriptor = async::get_filtered_descriptor(desc, columns_to_decode_); ranges_and_key_.col_range_.second = ranges_and_key_.col_range_.first + (descriptor.field_count() - descriptor.index().field_count()); ARCTICDB_TRACE(log::codec(), "Creating segment"); SegmentInMemory segment_in_memory(std::move(descriptor)); - decode_into_memory_segment(seg, hdr, segment_in_memory, desc); + decode_into_memory_segment(*seg, hdr, segment_in_memory, desc); return pipelines::SegmentAndSlice(std::move(ranges_and_key_), std::move(segment_in_memory)); } diff --git a/cpp/arcticdb/async/tasks.hpp b/cpp/arcticdb/async/tasks.hpp index f28c5ce2b3..8905a0744a 100644 --- a/cpp/arcticdb/async/tasks.hpp +++ b/cpp/arcticdb/async/tasks.hpp @@ -298,7 +298,12 @@ struct CopyCompressedTask : BaseTask { VariantKey copy() { return std::visit([that = this](const auto &source_key) { auto key_seg = that->lib_->read(source_key); - auto target_key_seg = stream::make_target_key(that->key_type_, that->stream_id_, that->version_id_, source_key, std::move(key_seg.segment())); + auto target_key_seg = stream::make_target_key( + that->key_type_, + that->stream_id_, + that->version_id_, + source_key, + std::move(*key_seg.release_segment())); auto return_key = target_key_seg.variant_key(); that->lib_->write(Composite{std::move(target_key_seg) }); return return_key; @@ -424,7 +429,7 @@ struct DecodeSegmentTask : BaseTask { ARCTICDB_DEBUG(log::storage(), "ReadAndDecodeAtomTask decoding segment with key {}", variant_key_view(key_seg.variant_key())); - return {key_seg.variant_key(), decode_segment(std::move(key_seg.segment()))}; + return {key_seg.variant_key(), decode_segment(*key_seg.segment_ptr())}; } }; @@ -546,7 +551,7 @@ struct DecodeTimeseriesDescriptorTask : BaseTask { auto key_seg = std::move(ks); ARCTICDB_DEBUG(log::storage(), "DecodeTimeseriesDescriptorTask decoding segment with key {}", variant_key_view(key_seg.variant_key())); - auto maybe_desc = decode_timeseries_descriptor(key_seg.segment()); + auto maybe_desc = decode_timeseries_descriptor(*key_seg.segment_ptr()); util::check(static_cast(maybe_desc), "Failed to decode timeseries descriptor"); return std::make_pair( @@ -567,7 +572,7 @@ struct DecodeMetadataAndDescriptorTask : BaseTask { auto key_seg = std::move(ks); ARCTICDB_DEBUG(log::storage(), "DecodeMetadataAndDescriptorTask decoding segment with key {}", variant_key_view(key_seg.variant_key())); - auto [any, descriptor] = decode_metadata_and_descriptor_fields(key_seg.segment()); + auto [any, descriptor] = decode_metadata_and_descriptor_fields(*key_seg.segment_ptr()); return std::make_tuple( std::move(key_seg.variant_key()), std::move(any), diff --git a/cpp/arcticdb/codec/codec.cpp b/cpp/arcticdb/codec/codec.cpp index 30f3c0e8d7..8a6cd6e8cf 100644 --- a/cpp/arcticdb/codec/codec.cpp +++ b/cpp/arcticdb/codec/codec.cpp @@ -557,8 +557,7 @@ void decode_into_memory_segment( decode_v1(segment, hdr, res, desc); } -SegmentInMemory decode_segment(Segment&& s) { - auto segment = std::move(s); +SegmentInMemory decode_segment(Segment& segment) { auto &hdr = segment.header(); ARCTICDB_TRACE(log::codec(), "Decoding descriptor: {}", segment.descriptor()); auto descriptor = segment.descriptor(); diff --git a/cpp/arcticdb/codec/codec.hpp b/cpp/arcticdb/codec/codec.hpp index 446bacaed2..b2ca6f00b3 100644 --- a/cpp/arcticdb/codec/codec.hpp +++ b/cpp/arcticdb/codec/codec.hpp @@ -57,7 +57,7 @@ EncodedFieldCollection decode_encoded_fields( const uint8_t* data, const uint8_t* begin ARCTICDB_UNUSED); -SegmentInMemory decode_segment(Segment&& segment); +SegmentInMemory decode_segment(Segment& segment); void decode_into_memory_segment( const Segment& segment, diff --git a/cpp/arcticdb/codec/python_bindings.cpp b/cpp/arcticdb/codec/python_bindings.cpp index 782b63d198..b2206cffc5 100644 --- a/cpp/arcticdb/codec/python_bindings.cpp +++ b/cpp/arcticdb/codec/python_bindings.cpp @@ -103,7 +103,7 @@ Segment encode_segment(SegmentInMemory segment_in_memory, const py::object &opts } SegmentInMemory decode_python_segment(Segment& segment) { - return decode_segment(std::move(segment)); + return decode_segment(segment); } class BufferPairDataSink { diff --git a/cpp/arcticdb/codec/test/test_codec.cpp b/cpp/arcticdb/codec/test/test_codec.cpp index dc8694b854..ac84522f80 100644 --- a/cpp/arcticdb/codec/test/test_codec.cpp +++ b/cpp/arcticdb/codec/test/test_codec.cpp @@ -319,7 +319,7 @@ TYPED_TEST(SegmentStringEncodingTest, EncodeSingleString) { constexpr EncodingVersion encoding_version = TypeParam::value; Segment seg = encode_dispatch(s.clone(), opt, encoding_version); - SegmentInMemory res = decode_segment(std::move(seg)); + SegmentInMemory res = decode_segment(seg); ASSERT_EQ(copy.string_at(0, 1), res.string_at(0, 1)); ASSERT_EQ(std::string("happy"), res.string_at(0, 1)); } @@ -346,7 +346,7 @@ TYPED_TEST(SegmentStringEncodingTest, EncodeStringsBasic) { constexpr EncodingVersion encoding_version = TypeParam::value; Segment seg = encode_dispatch(SegmentInMemory{s}, opt, encoding_version); - SegmentInMemory res = decode_segment(std::move(seg)); + SegmentInMemory res = decode_segment(seg); ASSERT_EQ(copy.string_at(0, 1), res.string_at(0, 1)); ASSERT_EQ(std::string("happy"), res.string_at(0, 1)); ASSERT_EQ(copy.string_at(1, 3), res.string_at(1, 3)); diff --git a/cpp/arcticdb/pipeline/read_frame.cpp b/cpp/arcticdb/pipeline/read_frame.cpp index de65763748..ca666b56f3 100644 --- a/cpp/arcticdb/pipeline/read_frame.cpp +++ b/cpp/arcticdb/pipeline/read_frame.cpp @@ -302,10 +302,9 @@ bool remaining_fields_empty(IteratorType it, const PipelineContextRow& context) void decode_into_frame_static( SegmentInMemory &frame, PipelineContextRow &context, - Segment &&s, + Segment& seg, const DecodePathData& shared_data, std::any& handler_data) { - auto seg = std::move(s); ARCTICDB_SAMPLE_DEFAULT(DecodeIntoFrame) const uint8_t *data = seg.buffer().data(); const uint8_t *begin = data; @@ -395,12 +394,11 @@ void decode_into_frame_static( void decode_into_frame_dynamic( SegmentInMemory& frame, PipelineContextRow& context, - Segment&& s, + Segment& seg, const DecodePathData& shared_data, std::any& handler_data ) { ARCTICDB_SAMPLE_DEFAULT(DecodeIntoFrame) - auto seg = std::move(s); const uint8_t *data = seg.buffer().data(); const uint8_t *begin = data; const uint8_t *end = begin + seg.buffer().bytes(); @@ -686,11 +684,10 @@ folly::Future fetch_data( [row=row, frame=frame, dynamic_schema=dynamic_schema, shared_data, &handler_data](auto &&ks) mutable { auto key_seg = std::forward(ks); if(dynamic_schema) { - decode_into_frame_dynamic(frame, row, std::move(key_seg.segment()), shared_data, handler_data); + decode_into_frame_dynamic(frame, row, *key_seg.segment_ptr(), shared_data, handler_data); } else { - decode_into_frame_static(frame, row, std::move(key_seg.segment()), shared_data, handler_data); + decode_into_frame_static(frame, row, *key_seg.segment_ptr(), shared_data, handler_data); } - return key_seg.variant_key(); }); } diff --git a/cpp/arcticdb/pipeline/read_frame.hpp b/cpp/arcticdb/pipeline/read_frame.hpp index b05868533b..81b91b5b5e 100644 --- a/cpp/arcticdb/pipeline/read_frame.hpp +++ b/cpp/arcticdb/pipeline/read_frame.hpp @@ -81,14 +81,14 @@ folly::Future fetch_data( void decode_into_frame_static( SegmentInMemory &frame, PipelineContextRow &context, - Segment &&seg, + Segment &seg, const DecodePathData& shared_data, std::any& handler_data); void decode_into_frame_dynamic( SegmentInMemory &frame, PipelineContextRow &context, - Segment &&seg, + Segment &seg, const DecodePathData& shared_data, std::any& handler_data); diff --git a/cpp/arcticdb/storage/azure/azure_client_wrapper.hpp b/cpp/arcticdb/storage/azure/azure_client_wrapper.hpp index bfae88ae64..3288effd84 100644 --- a/cpp/arcticdb/storage/azure/azure_client_wrapper.hpp +++ b/cpp/arcticdb/storage/azure/azure_client_wrapper.hpp @@ -50,7 +50,7 @@ class AzureClientWrapper { using Config = arcticdb::proto::azure_storage::Config; virtual void write_blob( const std::string& blob_name, - Segment&& segment, + Segment& segment, const Azure::Storage::Blobs::UploadBlockBlobFromOptions& upload_option, unsigned int request_timeout) = 0; diff --git a/cpp/arcticdb/storage/azure/azure_mock_client.cpp b/cpp/arcticdb/storage/azure/azure_mock_client.cpp index b22cacf3a3..15407a4c50 100644 --- a/cpp/arcticdb/storage/azure/azure_mock_client.cpp +++ b/cpp/arcticdb/storage/azure/azure_mock_client.cpp @@ -55,7 +55,7 @@ std::optional has_failure_trigger(const std void MockAzureClient::write_blob( const std::string& blob_name, - arcticdb::Segment&& segment, + arcticdb::Segment& segment, const Azure::Storage::Blobs::UploadBlockBlobFromOptions&, unsigned int) { diff --git a/cpp/arcticdb/storage/azure/azure_mock_client.hpp b/cpp/arcticdb/storage/azure/azure_mock_client.hpp index 147324f61e..eaa539f9c2 100644 --- a/cpp/arcticdb/storage/azure/azure_mock_client.hpp +++ b/cpp/arcticdb/storage/azure/azure_mock_client.hpp @@ -24,7 +24,7 @@ class MockAzureClient : public AzureClientWrapper { void write_blob( const std::string& blob_name, - Segment&& segment, + Segment& segment, const Azure::Storage::Blobs::UploadBlockBlobFromOptions& upload_option, unsigned int request_timeout) override; diff --git a/cpp/arcticdb/storage/azure/azure_real_client.cpp b/cpp/arcticdb/storage/azure/azure_real_client.cpp index 7da760dd38..c1ee9dc1fd 100644 --- a/cpp/arcticdb/storage/azure/azure_real_client.cpp +++ b/cpp/arcticdb/storage/azure/azure_real_client.cpp @@ -47,7 +47,7 @@ Azure::Storage::Blobs::BlobClientOptions RealAzureClient::get_client_options(con void RealAzureClient::write_blob( const std::string& blob_name, - Segment&& segment, + Segment& segment, const Azure::Storage::Blobs::UploadBlockBlobFromOptions& upload_option, unsigned int request_timeout) { diff --git a/cpp/arcticdb/storage/azure/azure_real_client.hpp b/cpp/arcticdb/storage/azure/azure_real_client.hpp index 9fc8116de0..e63017a4e1 100644 --- a/cpp/arcticdb/storage/azure/azure_real_client.hpp +++ b/cpp/arcticdb/storage/azure/azure_real_client.hpp @@ -26,7 +26,7 @@ class RealAzureClient : public AzureClientWrapper { void write_blob( const std::string& blob_name, - Segment&& segment, + Segment& segment, const Azure::Storage::Blobs::UploadBlockBlobFromOptions& upload_option, unsigned int request_timeout) override; diff --git a/cpp/arcticdb/storage/azure/azure_storage.cpp b/cpp/arcticdb/storage/azure/azure_storage.cpp index 1aef3183c5..5d8f887bc4 100644 --- a/cpp/arcticdb/storage/azure/azure_storage.cpp +++ b/cpp/arcticdb/storage/azure/azure_storage.cpp @@ -124,10 +124,9 @@ void do_write_impl( for (auto& kv : group.values()) { auto& k = kv.variant_key(); auto blob_name = object_path(b.bucketize(key_type_dir, k), k); - auto& seg = kv.segment(); try { - azure_client.write_blob(blob_name, std::move(seg), upload_option, request_timeout); + azure_client.write_blob(blob_name, *kv.segment_ptr(), upload_option, request_timeout); } catch (const Azure::Core::RequestFailedException& e) { raise_azure_exception(e, blob_name); diff --git a/cpp/arcticdb/storage/file/mapped_file_storage.cpp b/cpp/arcticdb/storage/file/mapped_file_storage.cpp index 3f04003bf2..4edb81bdf8 100644 --- a/cpp/arcticdb/storage/file/mapped_file_storage.cpp +++ b/cpp/arcticdb/storage/file/mapped_file_storage.cpp @@ -56,7 +56,7 @@ void MappedFileStorage::init() { SegmentInMemory MappedFileStorage::read_segment(size_t offset, size_t bytes) const { auto index_segment = Segment::from_bytes(file_.data() + offset, bytes); - return decode_segment(std::move(index_segment)); + return decode_segment(index_segment); } void MappedFileStorage::do_load_header(size_t header_offset, size_t header_size) { @@ -75,7 +75,7 @@ uint64_t MappedFileStorage::get_data_offset(const Segment& seg) { return previous_offset; } -uint64_t MappedFileStorage::write_segment(Segment&& seg) { +uint64_t MappedFileStorage::write_segment(Segment& seg) { auto segment = std::move(seg); auto offset = get_data_offset(segment); auto* data = file_.data() + offset; @@ -89,7 +89,7 @@ void MappedFileStorage::do_write(Composite&& kvs) { ARCTICDB_SAMPLE(MappedFileStorageWriteValues, 0) auto key_values = std::move(kvs); key_values.broadcast([this] (auto key_seg) { - const auto offset = write_segment(std::move(key_seg.segment())); + const auto offset = write_segment(*key_seg.segment_ptr()); const auto size = key_seg.segment().size(); multi_segment_header_.add_key_and_offset(key_seg.atom_key(), offset, size); }); @@ -129,7 +129,7 @@ void MappedFileStorage::do_finalize(KeyData key_data) { auto header_segment = encode_dispatch(multi_segment_header_.detach_segment(), config_.codec_opts(), EncodingVersion{static_cast(config_.encoding_version())}); - write_segment(std::move(header_segment)); + write_segment(header_segment); auto pos = reinterpret_cast(file_.data() + offset_); *pos = key_data; ARCTICDB_DEBUG(log::storage(), "Finalizing mapped file, writing key data {}", *pos); diff --git a/cpp/arcticdb/storage/file/mapped_file_storage.hpp b/cpp/arcticdb/storage/file/mapped_file_storage.hpp index ee4b395462..c0e3ee7f7e 100644 --- a/cpp/arcticdb/storage/file/mapped_file_storage.hpp +++ b/cpp/arcticdb/storage/file/mapped_file_storage.hpp @@ -70,7 +70,7 @@ class MappedFileStorage final : public SingleFileStorage { void do_load_header(size_t header_offset, size_t header_size) override; - uint64_t write_segment(Segment&& seg); + uint64_t write_segment(Segment& seg); uint8_t* do_read_raw(size_t offset, size_t bytes) override; diff --git a/cpp/arcticdb/storage/key_segment_pair.hpp b/cpp/arcticdb/storage/key_segment_pair.hpp index 7baa92e6ed..aba4c5ce33 100644 --- a/cpp/arcticdb/storage/key_segment_pair.hpp +++ b/cpp/arcticdb/storage/key_segment_pair.hpp @@ -35,13 +35,15 @@ namespace arcticdb::storage { ARCTICDB_MOVE_COPY_DEFAULT(KeySegmentPair) - // TODO aseaton remove - Segment& segment() { + std::unique_ptr release_segment() { util::check(segment_, "Attempting to access segment_ but it has not been set"); - return *segment_; + auto tmp = std::make_unique(std::move(*segment_)); + segment_ = std::make_shared(); + return tmp; } [[nodiscard]] std::shared_ptr segment_ptr() const { + util::check(segment_, "Attempting to access segment_ptr it is empty"); return segment_; } @@ -50,6 +52,10 @@ namespace arcticdb::storage { key_ = std::make_shared(std::forward(key)); } + void set_segment(Segment&& segment) { + segment_ = std::make_shared(std::move(segment)); + } + [[nodiscard]] const AtomKey &atom_key() const { util::check(std::holds_alternative(variant_key()), "Expected atom key access"); return std::get(variant_key()); diff --git a/cpp/arcticdb/storage/library.hpp b/cpp/arcticdb/storage/library.hpp index 6b3d58837c..e64b4cac6e 100644 --- a/cpp/arcticdb/storage/library.hpp +++ b/cpp/arcticdb/storage/library.hpp @@ -144,7 +144,7 @@ class Library { KeySegmentPair res{VariantKey{key}}; util::check(!std::holds_alternative(variant_key_id(key)) || !std::get(variant_key_id(key)).empty(), "Unexpected empty id"); const ReadVisitor& visitor = [&res](const VariantKey&, Segment&& value) { - res.segment() = std::move(value); + res.set_segment(std::move(value)); }; read(Composite(std::move(key)), visitor, opts); diff --git a/cpp/arcticdb/storage/lmdb/lmdb_client_wrapper.hpp b/cpp/arcticdb/storage/lmdb/lmdb_client_wrapper.hpp index e1860f3bf1..15a52c3f64 100644 --- a/cpp/arcticdb/storage/lmdb/lmdb_client_wrapper.hpp +++ b/cpp/arcticdb/storage/lmdb/lmdb_client_wrapper.hpp @@ -37,7 +37,7 @@ class LmdbClientWrapper { virtual void write( const std::string& db_name, std::string& path, - Segment&& segment, + Segment& segment, ::lmdb::txn& txn, ::lmdb::dbi& dbi, int64_t overwrite_flag) = 0; diff --git a/cpp/arcticdb/storage/lmdb/lmdb_mock_client.cpp b/cpp/arcticdb/storage/lmdb/lmdb_mock_client.cpp index 6c351d7932..de0db0990b 100644 --- a/cpp/arcticdb/storage/lmdb/lmdb_mock_client.cpp +++ b/cpp/arcticdb/storage/lmdb/lmdb_mock_client.cpp @@ -91,7 +91,7 @@ std::optional MockLmdbClient::read(const std::string& db_name, std::str return std::make_optional(lmdb_contents_.at(key).clone()); } -void MockLmdbClient::write(const std::string& db_name, std::string& path, arcticdb::Segment&& segment, +void MockLmdbClient::write(const std::string& db_name, std::string& path, arcticdb::Segment& segment, ::lmdb::txn&, ::lmdb::dbi&, int64_t) { LmdbKey key = {db_name, path}; raise_if_has_failure_trigger(key, StorageOperation::WRITE); @@ -99,7 +99,7 @@ void MockLmdbClient::write(const std::string& db_name, std::string& path, arctic if(has_key(key)) { raise_key_exists_error(lmdb_operation_string(StorageOperation::WRITE)); } else { - lmdb_contents_.try_emplace(key, std::move(segment)); + lmdb_contents_.try_emplace(key, segment.clone()); } } diff --git a/cpp/arcticdb/storage/lmdb/lmdb_mock_client.hpp b/cpp/arcticdb/storage/lmdb/lmdb_mock_client.hpp index 6e81bf6730..c4f3e38174 100644 --- a/cpp/arcticdb/storage/lmdb/lmdb_mock_client.hpp +++ b/cpp/arcticdb/storage/lmdb/lmdb_mock_client.hpp @@ -55,7 +55,7 @@ class MockLmdbClient : public LmdbClientWrapper { void write( const std::string& db_name, std::string& path, - Segment&& segment, + Segment& segment, ::lmdb::txn& txn, ::lmdb::dbi& dbi, int64_t overwrite_flag) override; diff --git a/cpp/arcticdb/storage/lmdb/lmdb_real_client.cpp b/cpp/arcticdb/storage/lmdb/lmdb_real_client.cpp index 547abcf114..1e53183684 100644 --- a/cpp/arcticdb/storage/lmdb/lmdb_real_client.cpp +++ b/cpp/arcticdb/storage/lmdb/lmdb_real_client.cpp @@ -39,7 +39,7 @@ std::optional RealLmdbClient::read(const std::string&, std::string& pat return segment; } -void RealLmdbClient::write(const std::string&, std::string& path, arcticdb::Segment&& seg, +void RealLmdbClient::write(const std::string&, std::string& path, arcticdb::Segment& seg, ::lmdb::txn& txn, ::lmdb::dbi& dbi, int64_t overwrite_flag) { MDB_val mdb_key{path.size(), path.data()}; diff --git a/cpp/arcticdb/storage/lmdb/lmdb_real_client.hpp b/cpp/arcticdb/storage/lmdb/lmdb_real_client.hpp index 279ed5f006..2f7a6ff5ab 100644 --- a/cpp/arcticdb/storage/lmdb/lmdb_real_client.hpp +++ b/cpp/arcticdb/storage/lmdb/lmdb_real_client.hpp @@ -34,7 +34,7 @@ class RealLmdbClient : public LmdbClientWrapper { void write( const std::string& db_name, std::string& path, - Segment&& segment, + Segment& segment, ::lmdb::txn& txn, ::lmdb::dbi& dbi, int64_t overwrite_flag) override; diff --git a/cpp/arcticdb/storage/lmdb/lmdb_storage.cpp b/cpp/arcticdb/storage/lmdb/lmdb_storage.cpp index fbd40a7503..cdf53b9267 100644 --- a/cpp/arcticdb/storage/lmdb/lmdb_storage.cpp +++ b/cpp/arcticdb/storage/lmdb/lmdb_storage.cpp @@ -92,10 +92,9 @@ void LmdbStorage::do_write_internal(Composite&& kvs, ::lmdb::txn for (auto &kv : group.values()) { ARCTICDB_DEBUG(log::storage(), "Lmdb storage writing segment with key {}", kv.key_view()); auto k = to_serialized_key(kv.variant_key()); - auto &seg = kv.segment(); int64_t overwrite_flag = std::holds_alternative(kv.variant_key()) ? 0 : MDB_NOOVERWRITE; try { - lmdb_client_->write(db_name, k, std::move(seg), txn, dbi, overwrite_flag); + lmdb_client_->write(db_name, k, *kv.segment_ptr(), txn, dbi, overwrite_flag); } catch (const ::lmdb::key_exist_error& e) { throw DuplicateKeyException(fmt::format("Key already exists: {}: {}", kv.variant_key(), e.what())); } catch (const ::lmdb::error& ex) { diff --git a/cpp/arcticdb/storage/memory/memory_storage.cpp b/cpp/arcticdb/storage/memory/memory_storage.cpp index ce180910dd..7788732809 100644 --- a/cpp/arcticdb/storage/memory/memory_storage.cpp +++ b/cpp/arcticdb/storage/memory/memory_storage.cpp @@ -18,11 +18,11 @@ namespace arcticdb::storage::memory { void add_serialization_fields(KeySegmentPair& kv) { - auto& segment = kv.segment(); - auto& hdr = segment.header(); - (void)segment.calculate_size(); + auto segment = kv.segment_ptr(); + auto& hdr = segment->header(); + (void)segment->calculate_size(); if(hdr.encoding_version() == EncodingVersion::V2) { - const auto* src = segment.buffer().data(); + const auto* src = segment->buffer().data(); set_body_fields(hdr, src); } } @@ -48,14 +48,14 @@ void add_serialization_fields(KeySegmentPair& kv) { key_vec.erase(it); } add_serialization_fields(kv); - key_vec.try_emplace(key, std::move(kv.segment())); + key_vec.try_emplace(key, std::move(*kv.release_segment())); }, [&](const AtomKey &key) { if (key_vec.find(key) != key_vec.end()) { throw DuplicateKeyException(key); } add_serialization_fields(kv); - key_vec.try_emplace(key, std::move(kv.segment())); + key_vec.try_emplace(key, std::move(*kv.release_segment())); } ); } diff --git a/cpp/arcticdb/storage/mongo/mongo_client.cpp b/cpp/arcticdb/storage/mongo/mongo_client.cpp index 5714c648af..fdbdb4589d 100644 --- a/cpp/arcticdb/storage/mongo/mongo_client.cpp +++ b/cpp/arcticdb/storage/mongo/mongo_client.cpp @@ -134,12 +134,11 @@ auto build_document(storage::KeySegmentPair &kv) { using builder::stream::document; const auto &key = kv.variant_key(); - auto &segment = kv.segment(); - const auto total_size = segment.calculate_size(); + const auto total_size = kv.segment_ptr()->calculate_size(); /*thread_local*/ std::vector buffer{}; buffer.resize(total_size); bsoncxx::types::b_binary data = {}; - kv.segment().write_to(buffer.data()); + kv.segment_ptr()->write_to(buffer.data()); data.size = uint32_t(total_size); data.bytes = buffer.data(); diff --git a/cpp/arcticdb/storage/mongo/mongo_mock_client.cpp b/cpp/arcticdb/storage/mongo/mongo_mock_client.cpp index 46ae0a39e3..8a3536a391 100644 --- a/cpp/arcticdb/storage/mongo/mongo_mock_client.cpp +++ b/cpp/arcticdb/storage/mongo/mongo_mock_client.cpp @@ -107,7 +107,7 @@ bool MockMongoClient::write_segment( return false; } - mongo_contents.insert_or_assign(std::move(key), std::move(kv.segment())); + mongo_contents.insert_or_assign(std::move(key), kv.segment().clone()); return true; } @@ -129,7 +129,7 @@ UpdateResult MockMongoClient::update_segment( return {0}; // upsert is false, don't update and return 0 as modified_count } - mongo_contents.insert_or_assign(std::move(key), std::move(kv.segment())); + mongo_contents.insert_or_assign(std::move(key), kv.segment().clone()); return {key_found ? 1 : 0}; } diff --git a/cpp/arcticdb/storage/mongo/mongo_storage.cpp b/cpp/arcticdb/storage/mongo/mongo_storage.cpp index b4511b83e9..5dd9bbfd01 100644 --- a/cpp/arcticdb/storage/mongo/mongo_storage.cpp +++ b/cpp/arcticdb/storage/mongo/mongo_storage.cpp @@ -136,7 +136,7 @@ void MongoStorage::do_read(Composite&& ks, const ReadVisitor& visito keys_not_found.push_back(k); } else { - visitor(k, std::move(kv->segment())); + visitor(k, std::move(*kv.value().release_segment())); } } catch (const mongocxx::operation_exception& ex) { diff --git a/cpp/arcticdb/storage/s3/detail-inl.hpp b/cpp/arcticdb/storage/s3/detail-inl.hpp index 5e9128e79f..99d2bf2e5e 100644 --- a/cpp/arcticdb/storage/s3/detail-inl.hpp +++ b/cpp/arcticdb/storage/s3/detail-inl.hpp @@ -120,9 +120,8 @@ namespace s3 { for (auto &kv: group.values()) { auto &k = kv.variant_key(); auto s3_object_name = object_path(b.bucketize(key_type_dir, k), k); - auto &seg = kv.segment(); - auto put_object_result = s3_client.put_object(s3_object_name, std::move(seg), bucket_name); + auto put_object_result = s3_client.put_object(s3_object_name, *kv.segment_ptr(), bucket_name); if (!put_object_result.is_success()) { auto& error = put_object_result.get_error(); @@ -144,9 +143,8 @@ namespace s3 { auto key_type_dir = key_type_folder(root_folder, kv.key_type()); auto &k = kv.variant_key(); auto s3_object_name = object_path(bucketizer.bucketize(key_type_dir, k), k); - auto &seg = kv.segment(); - auto put_object_result = s3_client.put_object(s3_object_name, std::move(seg), bucket_name, PutHeader::IF_NONE_MATCH); + auto put_object_result = s3_client.put_object(s3_object_name, *kv.segment_ptr(), bucket_name, PutHeader::IF_NONE_MATCH); if (!put_object_result.is_success()) { auto& error = put_object_result.get_error(); diff --git a/cpp/arcticdb/storage/s3/s3_client_wrapper.hpp b/cpp/arcticdb/storage/s3/s3_client_wrapper.hpp index 830629f44f..58268c61a1 100644 --- a/cpp/arcticdb/storage/s3/s3_client_wrapper.hpp +++ b/cpp/arcticdb/storage/s3/s3_client_wrapper.hpp @@ -61,7 +61,7 @@ class S3ClientWrapper { virtual S3Result put_object( const std::string& s3_object_name, - Segment&& segment, + Segment& segment, const std::string& bucket_name, PutHeader header = PutHeader::NONE) = 0; diff --git a/cpp/arcticdb/storage/s3/s3_mock_client.cpp b/cpp/arcticdb/storage/s3/s3_mock_client.cpp index 3d614d5a05..cc7d406de2 100644 --- a/cpp/arcticdb/storage/s3/s3_mock_client.cpp +++ b/cpp/arcticdb/storage/s3/s3_mock_client.cpp @@ -83,7 +83,7 @@ S3Result MockS3Client::get_object( S3Result MockS3Client::put_object( const std::string &s3_object_name, - Segment &&segment, + Segment &segment, const std::string &bucket_name, PutHeader header) { auto maybe_error = has_failure_trigger(s3_object_name, StorageOperation::WRITE); @@ -95,7 +95,7 @@ S3Result MockS3Client::put_object( return {precondition_failed_error}; } - s3_contents.insert_or_assign({bucket_name, s3_object_name}, std::move(segment)); + s3_contents.insert_or_assign({bucket_name, s3_object_name}, segment.clone()); return {std::monostate()}; } diff --git a/cpp/arcticdb/storage/s3/s3_mock_client.hpp b/cpp/arcticdb/storage/s3/s3_mock_client.hpp index 75561adb91..b1848a2e3a 100644 --- a/cpp/arcticdb/storage/s3/s3_mock_client.hpp +++ b/cpp/arcticdb/storage/s3/s3_mock_client.hpp @@ -60,7 +60,7 @@ class MockS3Client : public S3ClientWrapper { S3Result put_object( const std::string& s3_object_name, - Segment&& segment, + Segment& segment, const std::string& bucket_name, PutHeader header = PutHeader::NONE) override; diff --git a/cpp/arcticdb/storage/s3/s3_real_client.cpp b/cpp/arcticdb/storage/s3/s3_real_client.cpp index 2f5d6e79de..02468826c6 100644 --- a/cpp/arcticdb/storage/s3/s3_real_client.cpp +++ b/cpp/arcticdb/storage/s3/s3_real_client.cpp @@ -132,7 +132,7 @@ S3Result RealS3Client::get_object( S3Result RealS3Client::put_object( const std::string &s3_object_name, - Segment &&segment, + Segment &segment, const std::string &bucket_name, PutHeader header) { diff --git a/cpp/arcticdb/storage/s3/s3_real_client.hpp b/cpp/arcticdb/storage/s3/s3_real_client.hpp index 414178e507..f95dc1b63d 100644 --- a/cpp/arcticdb/storage/s3/s3_real_client.hpp +++ b/cpp/arcticdb/storage/s3/s3_real_client.hpp @@ -37,7 +37,7 @@ class RealS3Client : public S3ClientWrapper { S3Result put_object( const std::string& s3_object_name, - Segment&& segment, + Segment& segment, const std::string& bucket_name, PutHeader header = PutHeader::NONE) override; diff --git a/cpp/arcticdb/storage/storage.hpp b/cpp/arcticdb/storage/storage.hpp index 27abfb5b23..0974698b73 100644 --- a/cpp/arcticdb/storage/storage.hpp +++ b/cpp/arcticdb/storage/storage.hpp @@ -133,7 +133,7 @@ class Storage { KeySegmentPair key_seg; const ReadVisitor& visitor = [&key_seg](const VariantKey & vk, Segment&& value) { key_seg.set_key(vk); - key_seg.segment() = std::move(value); + key_seg.set_segment(std::move(value)); }; read(std::forward(key), visitor, opts); diff --git a/cpp/arcticdb/toolbox/library_tool.cpp b/cpp/arcticdb/toolbox/library_tool.cpp index ecd0dcc57b..7bde5ac275 100644 --- a/cpp/arcticdb/toolbox/library_tool.cpp +++ b/cpp/arcticdb/toolbox/library_tool.cpp @@ -37,7 +37,7 @@ async::AsyncStore<>& LibraryTool::async_store() { ReadResult LibraryTool::read(const VariantKey& key) { auto segment = read_to_segment(key); - auto segment_in_memory = decode_segment(std::move(segment)); + auto segment_in_memory = decode_segment(segment); auto frame_and_descriptor = frame_and_descriptor_from_segment(std::move(segment_in_memory)); auto atom_key = util::variant_match( key, @@ -51,8 +51,8 @@ ReadResult LibraryTool::read(const VariantKey& key) { Segment LibraryTool::read_to_segment(const VariantKey& key) { auto kv = store()->read_compressed_sync(key, storage::ReadKeyOpts{}); util::check(kv.has_segment(), "Failed to read key: {}", key); - kv.segment().force_own_buffer(); - return std::move(kv.segment()); + kv.segment_ptr()->force_own_buffer(); + return std::move(*kv.release_segment()); } std::optional LibraryTool::read_metadata(const VariantKey& key){ @@ -88,7 +88,7 @@ SegmentInMemory LibraryTool::overwrite_append_data( std::holds_alternative(key) && std::get(key).type() == KeyType::APPEND_DATA, "Can only override APPEND_DATA keys. Received: {}", key); auto old_segment = read_to_segment(key); - auto old_segment_in_memory = decode_segment(std::move(old_segment)); + auto old_segment_in_memory = decode_segment(old_segment); const auto& tsd = old_segment_in_memory.index_descriptor(); std::optional next_key = std::nullopt; if (tsd.proto().has_next_key()){