From 781514acfeb146df5201fe24e58cda04cb8c7b73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Chojnowski?= Date: Wed, 22 Feb 2023 10:37:01 +0100 Subject: [PATCH] mutation_partition_v2: change schema_ptr to schema& in mutation_partition_v2 constructor We don't have a convention for when to pass `schema_ptr` and and when to pass `const schema&` around. In general, IMHO the natural convention for such a situation is to pass the shared pointer if the callee might extend the lifetime of shared_ptr, and pass a reference otherwise. But we convert between them willy-nilly through shared_from_this(). While passing a reference to a function which actually expects a shared_ptr can make sense (e.g. due to the fact that smart pointers can't be passed in registers), the other way around is rather pointless. This patch takes one occurence of that and modifies the parameter to a reference. Since enable_shared_from_this makes shared pointer parameters and reference parameters interchangeable, this is a purely cosmetic change. --- mutation/mutation_partition_v2.cc | 2 +- mutation/mutation_partition_v2.hh | 4 ++-- mutation/partition_version.cc | 4 ++-- mutation/partition_version.hh | 2 +- test/boost/mutation_test.cc | 2 +- test/boost/mvcc_test.cc | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/mutation/mutation_partition_v2.cc b/mutation/mutation_partition_v2.cc index 6f9a5b04ff..efac0fa8a7 100644 --- a/mutation/mutation_partition_v2.cc +++ b/mutation/mutation_partition_v2.cc @@ -66,7 +66,7 @@ mutation_partition_v2::mutation_partition_v2(const schema& s, mutation_partition auto&& tombstones = x.mutable_row_tombstones(); if (!tombstones.empty()) { try { - mutation_partition_v2 p(s.shared_from_this()); + mutation_partition_v2 p(s); for (auto&& t: tombstones) { range_tombstone & rt = t.tombstone(); diff --git a/mutation/mutation_partition_v2.hh b/mutation/mutation_partition_v2.hh index fb5a5efe31..f5c74971cc 100644 --- a/mutation/mutation_partition_v2.hh +++ b/mutation/mutation_partition_v2.hh @@ -89,10 +89,10 @@ public: static mutation_partition_v2 make_incomplete(const schema& s, tombstone t = {}) { return mutation_partition_v2(incomplete_tag(), s, t); } - mutation_partition_v2(schema_ptr s) + mutation_partition_v2(const schema& s) : _rows() #ifdef SEASTAR_DEBUG - , _schema_version(s->version()) + , _schema_version(s.version()) #endif { } mutation_partition_v2(mutation_partition_v2& other, copy_comparators_only) diff --git a/mutation/partition_version.cc b/mutation/partition_version.cc index cbe48d7c71..fb40638642 100644 --- a/mutation/partition_version.cc +++ b/mutation/partition_version.cc @@ -329,7 +329,7 @@ partition_version& partition_entry::add_version(const schema& s, cache_tracker* // Such versions must be fully discontinuous, and thus have a dummy at the end. auto new_version = tracker ? current_allocator().construct(mutation_partition_v2::make_incomplete(s)) - : current_allocator().construct(mutation_partition_v2(s.shared_from_this())); + : current_allocator().construct(mutation_partition_v2(s)); new_version->partition().set_static_row_continuous(_version->partition().static_row_continuous()); new_version->insert_before(*_version); set_version(new_version); @@ -548,7 +548,7 @@ utils::coroutine partition_entry::apply_to_incomplete(const schema& s, mutation_partition_v2 partition_entry::squashed(schema_ptr from, schema_ptr to, is_evictable evictable) { - mutation_partition_v2 mp(to); + mutation_partition_v2 mp(*to); mp.set_static_row_continuous(_version->partition().static_row_continuous()); for (auto&& v : _version->all_elements()) { auto older = mutation_partition_v2(*from, v.partition()); diff --git a/mutation/partition_version.hh b/mutation/partition_version.hh index f3295be1e7..53de3fbcf7 100644 --- a/mutation/partition_version.hh +++ b/mutation/partition_version.hh @@ -120,7 +120,7 @@ public: } explicit partition_version(schema_ptr s) noexcept - : _partition(std::move(s)) { } + : _partition(*s) { } explicit partition_version(mutation_partition_v2 mp) noexcept : _partition(std::move(mp)) { } partition_version(partition_version&& pv) noexcept; diff --git a/test/boost/mutation_test.cc b/test/boost/mutation_test.cc index 53fb232ecb..0a91946e48 100644 --- a/test/boost/mutation_test.cc +++ b/test/boost/mutation_test.cc @@ -2132,7 +2132,7 @@ SEASTAR_TEST_CASE(test_v2_merging_in_non_evictable_snapshot) { static void clear(cache_tracker& tracker, const schema& s, mutation_partition_v2& p) { while (p.clear_gently(&tracker) == stop_iteration::no) {} - p = mutation_partition_v2(s.shared_from_this()); + p = mutation_partition_v2(s); tracker.insert(p); } diff --git a/test/boost/mvcc_test.cc b/test/boost/mvcc_test.cc index d35477baf6..b414fc36f0 100644 --- a/test/boost/mvcc_test.cc +++ b/test/boost/mvcc_test.cc @@ -1173,7 +1173,7 @@ public: , _tracker(t) , _r(r) , _e(_tracker ? partition_entry::make_evictable(*_schema, mutation_partition::make_incomplete(*_schema)) - : partition_entry(mutation_partition_v2(_schema))) + : partition_entry(mutation_partition_v2(*_schema))) { if (_tracker) { _tracker->insert(_e);