]> git-server-git.apps.pok.os.sepia.ceph.com Git - rocksdb.git/commitdiff
Replace ScopedArenaIterator with ScopedArenaPtr<InternalIterator> (#12470)
authorPeter Dillinger <peterd@meta.com>
Fri, 22 Mar 2024 20:40:42 +0000 (13:40 -0700)
committerFacebook GitHub Bot <facebook-github-bot@users.noreply.github.com>
Fri, 22 Mar 2024 20:40:42 +0000 (13:40 -0700)
Summary:
ScopedArenaIterator is not an iterator. It is a pointer wrapper. And we don't need a custom implemented pointer wrapper when std::unique_ptr can be instantiated with what we want.

So this adds ScopedArenaPtr<T> to replace those uses.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/12470

Test Plan: CI (including ASAN/UBSAN)

Reviewed By: jowlyzhang

Differential Revision: D55254362

Pulled By: pdillinger

fbshipit-source-id: cc96a0b9840df99aa807f417725e120802c0ae18

27 files changed:
db/builder.h
db/column_family.cc
db/compaction/compaction_job.h
db/compaction/compaction_job_stats_test.cc
db/db_compaction_filter_test.cc
db/db_impl/db_impl.h
db/db_impl/db_impl_open.cc
db/db_range_del_test.cc
db/db_test.cc
db/db_test_util.cc
db/db_test_util.h
db/external_sst_file_ingestion_job.cc
db/flush_job.cc
db/flush_job.h
db/import_column_family_job.cc
db/range_del_aggregator.cc
db/range_del_aggregator.h
db/repair.cc
db/version_set.cc
db/write_batch_test.cc
java/rocksjni/write_batch.cc
java/rocksjni/write_batch_test.cc
memory/arena.h
table/scoped_arena_iterator.h [deleted file]
table/table_test.cc
tools/ldb_cmd.cc
utilities/debug.cc

index 6e6bc63c51093d855f8f0b47492a87465c6e6059..f228f8d0fe729fc49107931eee743f7d64254e8a 100644 (file)
@@ -23,7 +23,6 @@
 #include "rocksdb/status.h"
 #include "rocksdb/table_properties.h"
 #include "rocksdb/types.h"
-#include "table/scoped_arena_iterator.h"
 
 namespace ROCKSDB_NAMESPACE {
 
index 73e695beca8cf3832fe8002e3bc49acd19b768de..2b606ec4095de3c758721a02089f948b77bef7d1 100644 (file)
@@ -1204,7 +1204,7 @@ Status ColumnFamilyData::RangesOverlapWithMemtables(
   super_version->imm->AddIterators(read_opts, /*seqno_to_time_mapping=*/nullptr,
                                    &merge_iter_builder,
                                    false /* add_range_tombstone_iter */);
-  ScopedArenaIterator memtable_iter(merge_iter_builder.Finish());
+  ScopedArenaPtr<InternalIterator> memtable_iter(merge_iter_builder.Finish());
 
   auto read_seq = super_version->current->version_set()->LastSequence();
   ReadRangeDelAggregator range_del_agg(&internal_comparator_, read_seq);
index e812cfc72a30050f2b27898f5eb926bb15d8236f..caa1593e72dd359772a137fb23581fe31820c689 100644 (file)
@@ -41,7 +41,6 @@
 #include "rocksdb/env.h"
 #include "rocksdb/memtablerep.h"
 #include "rocksdb/transaction_log.h"
-#include "table/scoped_arena_iterator.h"
 #include "util/autovector.h"
 #include "util/stop_watch.h"
 #include "util/thread_local.h"
index 1cc6b31148b7024f56e8480b57a223d8b0985a6b..690994519769c043e90b6e843831cc7fd2168b22 100644 (file)
@@ -46,7 +46,6 @@
 #include "table/block_based/block_based_table_factory.h"
 #include "table/mock_table.h"
 #include "table/plain/plain_table_factory.h"
-#include "table/scoped_arena_iterator.h"
 #include "test_util/sync_point.h"
 #include "test_util/testharness.h"
 #include "test_util/testutil.h"
index 6cedb6fd5304e3abe45f955e899a690f8d0186fa..d84e24c41cbccc4c22dd1dd2980ad89c053c0457 100644 (file)
@@ -342,7 +342,7 @@ TEST_F(DBTestCompactionFilter, CompactionFilter) {
   {
     InternalKeyComparator icmp(options.comparator);
     ReadOptions read_options;
-    ScopedArenaIterator iter(dbfull()->NewInternalIterator(
+    ScopedArenaPtr<InternalIterator> iter(dbfull()->NewInternalIterator(
         read_options, &arena, kMaxSequenceNumber, handles_[1]));
     iter->SeekToFirst();
     ASSERT_OK(iter->status());
@@ -434,7 +434,7 @@ TEST_F(DBTestCompactionFilter, CompactionFilter) {
   {
     InternalKeyComparator icmp(options.comparator);
     ReadOptions read_options;
-    ScopedArenaIterator iter(dbfull()->NewInternalIterator(
+    ScopedArenaPtr<InternalIterator> iter(dbfull()->NewInternalIterator(
         read_options, &arena, kMaxSequenceNumber, handles_[1]));
     iter->SeekToFirst();
     ASSERT_OK(iter->status());
@@ -717,8 +717,8 @@ TEST_F(DBTestCompactionFilter, CompactionFilterContextManual) {
     Arena arena;
     InternalKeyComparator icmp(options.comparator);
     ReadOptions read_options;
-    ScopedArenaIterator iter(dbfull()->NewInternalIterator(read_options, &arena,
-                                                           kMaxSequenceNumber));
+    ScopedArenaPtr<InternalIterator> iter(dbfull()->NewInternalIterator(
+        read_options, &arena, kMaxSequenceNumber));
     iter->SeekToFirst();
     ASSERT_OK(iter->status());
     while (iter->Valid()) {
index a7181c9e0f34ebc157dd246f93661c5d6f1c5fcb..244d979c1a5694fd721b8536e4815902f46ca9f5 100644 (file)
@@ -59,7 +59,6 @@
 #include "rocksdb/utilities/replayer.h"
 #include "rocksdb/write_buffer_manager.h"
 #include "table/merging_iterator.h"
-#include "table/scoped_arena_iterator.h"
 #include "util/autovector.h"
 #include "util/hash.h"
 #include "util/repeatable_thread.h"
index 786abb74f24de6aa05ffa61f6a44d257d0fa35fc..a0e3f7cf15237cdf7268eb51030397c71557ec7c 100644 (file)
@@ -1641,7 +1641,7 @@ Status DBImpl::WriteLevel0TableForRecovery(int job_id, ColumnFamilyData* cfd,
   Status s;
   TableProperties table_properties;
   {
-    ScopedArenaIterator iter(
+    ScopedArenaPtr<InternalIterator> iter(
         mem->NewIterator(ro, /*seqno_to_time_mapping=*/nullptr, &arena));
     ROCKS_LOG_DEBUG(immutable_db_options_.info_log,
                     "[%s] [WriteLevel0TableForRecovery]"
index b5d63a2298918c48856d98f3748f2d9812150755..f92fa27aed5a275537052d012aa47f1a15e5c429 100644 (file)
@@ -2833,8 +2833,8 @@ TEST_F(DBRangeDelTest, LeftSentinelKeyTestWithNewerKey) {
   Arena arena;
   InternalKeyComparator icmp(options.comparator);
   ReadOptions read_options;
-  ScopedArenaIterator iter;
-  iter.set(
+  ScopedArenaPtr<InternalIterator> iter;
+  iter.reset(
       dbfull()->NewInternalIterator(read_options, &arena, kMaxSequenceNumber));
 
   auto k = Key(4);
index b2c6aab6d679483533b8b0024bc3412020421878..3d514e39ad2e1d8c51bc4ae3fa09e747377a6f3d 100644 (file)
@@ -59,7 +59,6 @@
 #include "rocksdb/utilities/optimistic_transaction_db.h"
 #include "rocksdb/utilities/write_batch_with_index.h"
 #include "table/mock_table.h"
-#include "table/scoped_arena_iterator.h"
 #include "test_util/sync_point.h"
 #include "test_util/testharness.h"
 #include "test_util/testutil.h"
index fd751782d1404708b78e6947ccbde95327edf99e..cbc394b0426f911d8e7163e3b65b06a20aa90784 100644 (file)
@@ -992,13 +992,13 @@ std::string DBTestBase::AllEntriesFor(const Slice& user_key, int cf) {
   auto options = CurrentOptions();
   InternalKeyComparator icmp(options.comparator);
   ReadOptions read_options;
-  ScopedArenaIterator iter;
+  ScopedArenaPtr<InternalIterator> iter;
   if (cf == 0) {
-    iter.set(dbfull()->NewInternalIterator(read_options, &arena,
-                                           kMaxSequenceNumber));
+    iter.reset(dbfull()->NewInternalIterator(read_options, &arena,
+                                             kMaxSequenceNumber));
   } else {
-    iter.set(dbfull()->NewInternalIterator(read_options, &arena,
-                                           kMaxSequenceNumber, handles_[cf]));
+    iter.reset(dbfull()->NewInternalIterator(read_options, &arena,
+                                             kMaxSequenceNumber, handles_[cf]));
   }
   InternalKey target(user_key, kMaxSequenceNumber, kTypeValue);
   iter->Seek(target.Encode());
@@ -1471,13 +1471,13 @@ void DBTestBase::validateNumberOfEntries(int numValues, int cf) {
   auto options = CurrentOptions();
   InternalKeyComparator icmp(options.comparator);
   ReadOptions read_options;
-  ScopedArenaIterator iter;
+  ScopedArenaPtr<InternalIterator> iter;
   if (cf != 0) {
-    iter.set(dbfull()->NewInternalIterator(read_options, &arena,
-                                           kMaxSequenceNumber, handles_[cf]));
+    iter.reset(dbfull()->NewInternalIterator(read_options, &arena,
+                                             kMaxSequenceNumber, handles_[cf]));
   } else {
-    iter.set(dbfull()->NewInternalIterator(read_options, &arena,
-                                           kMaxSequenceNumber));
+    iter.reset(dbfull()->NewInternalIterator(read_options, &arena,
+                                             kMaxSequenceNumber));
   }
   iter->SeekToFirst();
   ASSERT_OK(iter->status());
index 2a671e1e8cff5a552928d05ea201ab4f7a726dbe..775c161d36bc24aafa9cebccfe5c50ef810a7952 100644 (file)
@@ -41,7 +41,6 @@
 #include "rocksdb/table.h"
 #include "rocksdb/utilities/checkpoint.h"
 #include "table/mock_table.h"
-#include "table/scoped_arena_iterator.h"
 #include "test_util/sync_point.h"
 #include "test_util/testharness.h"
 #include "util/cast_util.h"
index a671eea5e131abe6ce2ae6234247b6f42e24cd88..684952c98ece6f9c5e32b60ed903e9a2720c7921 100644 (file)
@@ -17,7 +17,6 @@
 #include "file/random_access_file_reader.h"
 #include "logging/logging.h"
 #include "table/merging_iterator.h"
-#include "table/scoped_arena_iterator.h"
 #include "table/sst_file_writer_collectors.h"
 #include "table/table_builder.h"
 #include "table/unique_id_impl.h"
index 7995ea786e58b7921f3476b57cfcf214491c365c..1a317f3b23afd8e1d61cee9259850ad329fb798c 100644 (file)
@@ -440,7 +440,7 @@ Status FlushJob::MemPurge() {
                          : earliest_seqno;
   }
 
-  ScopedArenaIterator iter(
+  ScopedArenaPtr<InternalIterator> iter(
       NewMergingIterator(&(cfd_->internal_comparator()), memtables.data(),
                          static_cast<int>(memtables.size()), &arena));
 
@@ -917,7 +917,7 @@ Status FlushJob::WriteLevel0Table() {
                          << GetFlushReasonString(flush_reason_);
 
     {
-      ScopedArenaIterator iter(
+      ScopedArenaPtr<InternalIterator> iter(
           NewMergingIterator(&cfd_->internal_comparator(), memtables.data(),
                              static_cast<int>(memtables.size()), &arena));
       ROCKS_LOG_INFO(db_options_.info_log,
index 9e636e6a85766db537bf8208829356cd62b29639..337e9cd9bc19d4c60df1515987e52203f0ba39ac 100644 (file)
@@ -39,7 +39,6 @@
 #include "rocksdb/listener.h"
 #include "rocksdb/memtablerep.h"
 #include "rocksdb/transaction_log.h"
-#include "table/scoped_arena_iterator.h"
 #include "util/autovector.h"
 #include "util/stop_watch.h"
 #include "util/thread_local.h"
index c40bb0cab9f565b27c57fd3fc8b0da8387754e31..4d5c65616e1a4df68c6c9995439d46470b95c0c8 100644 (file)
@@ -18,7 +18,6 @@
 #include "file/random_access_file_reader.h"
 #include "logging/logging.h"
 #include "table/merging_iterator.h"
-#include "table/scoped_arena_iterator.h"
 #include "table/sst_file_writer_collectors.h"
 #include "table/table_builder.h"
 #include "table/unique_id_impl.h"
index 652afe65a9a5bfdeaebe7a959018b514514cc0c3..f41521e1162aff2226437f881f63c338072c0a3d 100644 (file)
@@ -13,7 +13,6 @@
 #include "rocksdb/comparator.h"
 #include "rocksdb/types.h"
 #include "table/internal_iterator.h"
-#include "table/scoped_arena_iterator.h"
 #include "table/table_builder.h"
 #include "util/heap.h"
 #include "util/kv_map.h"
index f7fa87af40ddeed4729b007d919d4c4f1e76cf4e..f367a26787e53679379de38a4cfdf3ca117bc816 100644 (file)
@@ -22,7 +22,6 @@
 #include "rocksdb/comparator.h"
 #include "rocksdb/types.h"
 #include "table/internal_iterator.h"
-#include "table/scoped_arena_iterator.h"
 #include "table/table_builder.h"
 #include "util/heap.h"
 #include "util/kv_map.h"
index 34d79c5df5514ff031363235034345f211fa05f8..4fe8b478863f40f8f1be7737d3320a49cd0ba5e8 100644 (file)
@@ -81,7 +81,6 @@
 #include "rocksdb/env.h"
 #include "rocksdb/options.h"
 #include "rocksdb/write_buffer_manager.h"
-#include "table/scoped_arena_iterator.h"
 #include "table/unique_id_impl.h"
 #include "util/string_util.h"
 
@@ -443,7 +442,7 @@ class Repairer {
       ReadOptions ro;
       ro.total_order_seek = true;
       Arena arena;
-      ScopedArenaIterator iter(
+      ScopedArenaPtr<InternalIterator> iter(
           mem->NewIterator(ro, /*seqno_to_time_mapping=*/nullptr, &arena));
       int64_t _current_time = 0;
       immutable_db_options_.clock->GetCurrentTime(&_current_time)
index 1b36ad34cc19f32e3c8eb72e284729d0d3b058dc..23f6d770f6cad5298522b8620731ac0788110cfc 100644 (file)
@@ -2104,7 +2104,7 @@ Status Version::OverlapWithLevelIterator(const ReadOptions& read_options,
           BeforeFile(ucmp, &largest_user_key, file)) {
         continue;
       }
-      ScopedArenaIterator iter(cfd_->table_cache()->NewIterator(
+      ScopedArenaPtr<InternalIterator> iter(cfd_->table_cache()->NewIterator(
           read_options, file_options, cfd_->internal_comparator(),
           *file->file_metadata, &range_del_agg,
           mutable_cf_options_.prefix_extractor, nullptr,
@@ -2123,7 +2123,7 @@ Status Version::OverlapWithLevelIterator(const ReadOptions& read_options,
     }
   } else if (storage_info_.LevelFilesBrief(level).num_files > 0) {
     auto mem = arena.AllocateAligned(sizeof(LevelIterator));
-    ScopedArenaIterator iter(new (mem) LevelIterator(
+    ScopedArenaPtr<InternalIterator> iter(new (mem) LevelIterator(
         cfd_->table_cache(), read_options, file_options,
         cfd_->internal_comparator(), &storage_info_.LevelFilesBrief(level),
         mutable_cf_options_.prefix_extractor, should_sample_file_read(),
index 05aec899a561c905d3755aa99f883d031c338947..8db8c32a0a83aa98fdce2b14a150ffa3912af98a 100644 (file)
@@ -21,7 +21,6 @@
 #include "rocksdb/memtablerep.h"
 #include "rocksdb/utilities/write_batch_with_index.h"
 #include "rocksdb/write_buffer_manager.h"
-#include "table/scoped_arena_iterator.h"
 #include "test_util/testharness.h"
 #include "test_util/testutil.h"
 #include "util/string_util.h"
@@ -55,13 +54,13 @@ static std::string PrintContents(WriteBatch* b,
   int merge_count = 0;
   for (int i = 0; i < 2; ++i) {
     Arena arena;
-    ScopedArenaIterator arena_iter_guard;
+    ScopedArenaPtr<InternalIterator> arena_iter_guard;
     std::unique_ptr<InternalIterator> iter_guard;
     InternalIterator* iter;
     if (i == 0) {
       iter = mem->NewIterator(ReadOptions(), /*seqno_to_time_mapping=*/nullptr,
                               &arena);
-      arena_iter_guard.set(iter);
+      arena_iter_guard.reset(iter);
     } else {
       iter = mem->NewRangeTombstoneIterator(ReadOptions(),
                                             kMaxSequenceNumber /* read_seq */,
index 8ead84671d98de9c940d08b7c31b647588f7a2d9..f2f13b6b757e8355d7507d6bf31f0da1549918db 100644 (file)
@@ -22,7 +22,6 @@
 #include "rocksjni/cplusplus_to_java_convert.h"
 #include "rocksjni/portal.h"
 #include "rocksjni/writebatchhandlerjnicallback.h"
-#include "table/scoped_arena_iterator.h"
 
 /*
  * Class:     org_rocksdb_WriteBatch
index f3163374cb502f8e491ff714f9e665d48e3f0a84..53f10998ca86e771ee82781c825f5dbf131a807d 100644 (file)
@@ -22,7 +22,6 @@
 #include "rocksdb/status.h"
 #include "rocksdb/write_buffer_manager.h"
 #include "rocksjni/portal.h"
-#include "table/scoped_arena_iterator.h"
 #include "test_util/testharness.h"
 #include "util/string_util.h"
 
@@ -59,7 +58,7 @@ jbyteArray Java_org_rocksdb_WriteBatchTest_getContents(JNIEnv* env,
                                                         nullptr, nullptr);
   unsigned int count = 0;
   ROCKSDB_NAMESPACE::Arena arena;
-  ROCKSDB_NAMESPACE::ScopedArenaIterator iter(
+  ROCKSDB_NAMESPACE::ScopedArenaPtr<ROCKSDB_NAMESPACE::InternalIterator> iter(
       mem->NewIterator(ROCKSDB_NAMESPACE::ReadOptions(),
                        /*seqno_to_time_mapping=*/nullptr, &arena));
   for (iter->SeekToFirst(); iter->Valid(); iter->Next()) {
index 39399aa71b4f5ffb850c82453b0d824768ffa5b1..2257a4935dd02a102591a8ba01af74f6758986df 100644 (file)
@@ -132,4 +132,15 @@ inline char* Arena::Allocate(size_t bytes) {
   return AllocateFallback(bytes, false /* unaligned */);
 }
 
+// Like std::destroy_at but a callable type
+template <typename T>
+struct Destroyer {
+  void operator()(T* ptr) { ptr->~T(); }
+};
+
+// Like std::unique_ptr but only placement-deletes the object (for
+// objects allocated on an arena).
+template <typename T>
+using ScopedArenaPtr = std::unique_ptr<T, Destroyer<T>>;
+
 }  // namespace ROCKSDB_NAMESPACE
diff --git a/table/scoped_arena_iterator.h b/table/scoped_arena_iterator.h
deleted file mode 100644 (file)
index 2b8824d..0000000
+++ /dev/null
@@ -1,57 +0,0 @@
-// Copyright (c) 2011-present, Facebook, Inc.  All rights reserved.
-//  This source code is licensed under both the GPLv2 (found in the
-//  COPYING file in the root directory) and Apache 2.0 License
-//  (found in the LICENSE.Apache file in the root directory).
-// Copyright (c) 2011 The LevelDB Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file. See the AUTHORS file for names of contributors.
-#pragma once
-
-#include "port/port.h"
-#include "table/internal_iterator.h"
-
-namespace ROCKSDB_NAMESPACE {
-class ScopedArenaIterator {
-  void reset(InternalIterator* iter) noexcept {
-    if (iter_ != nullptr) {
-      iter_->~InternalIterator();
-    }
-    iter_ = iter;
-  }
-
- public:
-  explicit ScopedArenaIterator(InternalIterator* iter = nullptr)
-      : iter_(iter) {}
-
-  ScopedArenaIterator(const ScopedArenaIterator&) = delete;
-  ScopedArenaIterator& operator=(const ScopedArenaIterator&) = delete;
-
-  ScopedArenaIterator(ScopedArenaIterator&& o) noexcept {
-    iter_ = o.iter_;
-    o.iter_ = nullptr;
-  }
-
-  ScopedArenaIterator& operator=(ScopedArenaIterator&& o) noexcept {
-    reset(o.iter_);
-    o.iter_ = nullptr;
-    return *this;
-  }
-
-  InternalIterator* operator->() { return iter_; }
-  InternalIterator* get() { return iter_; }
-
-  void set(InternalIterator* iter) { reset(iter); }
-
-  InternalIterator* release() {
-    assert(iter_ != nullptr);
-    auto* res = iter_;
-    iter_ = nullptr;
-    return res;
-  }
-
-  ~ScopedArenaIterator() { reset(nullptr); }
-
- private:
-  InternalIterator* iter_;
-};
-}  // namespace ROCKSDB_NAMESPACE
index 432799468beba4d98f9e657bde53b4bd2b599430..02a8d899d7790153bd5f4bd8829ba9e7981ec537 100644 (file)
@@ -63,7 +63,6 @@
 #include "table/internal_iterator.h"
 #include "table/meta_blocks.h"
 #include "table/plain/plain_table_factory.h"
-#include "table/scoped_arena_iterator.h"
 #include "table/sst_file_writer_collectors.h"
 #include "table/unique_id_impl.h"
 #include "test_util/sync_point.h"
@@ -4895,13 +4894,13 @@ TEST_F(MemTableTest, Simple) {
 
   for (int i = 0; i < 2; ++i) {
     Arena arena;
-    ScopedArenaIterator arena_iter_guard;
+    ScopedArenaPtr<InternalIterator> arena_iter_guard;
     std::unique_ptr<InternalIterator> iter_guard;
     InternalIterator* iter;
     if (i == 0) {
       iter = GetMemTable()->NewIterator(
           ReadOptions(), /*seqno_to_time_mapping=*/nullptr, &arena);
-      arena_iter_guard.set(iter);
+      arena_iter_guard.reset(iter);
     } else {
       iter = GetMemTable()->NewRangeTombstoneIterator(
           ReadOptions(), kMaxSequenceNumber /* read_seq */,
index dcc03347b8d88e31e4f31f7ec4cfbd994746ea31..355e26fa7c07674986e2164df70082972d94be6f 100644 (file)
@@ -38,7 +38,6 @@
 #include "rocksdb/utilities/options_util.h"
 #include "rocksdb/write_batch.h"
 #include "rocksdb/write_buffer_manager.h"
-#include "table/scoped_arena_iterator.h"
 #include "table/sst_file_dumper.h"
 #include "tools/ldb_cmd_impl.h"
 #include "util/cast_util.h"
index c7599797dad64daabc2b011ebcdbeaaa39869cc3..6274cd6c2d0970321a51c290c773e2e85fe08f95 100644 (file)
@@ -82,7 +82,7 @@ Status GetAllKeyVersions(DB* db, ColumnFamilyHandle* cfh, Slice begin_key,
   auto icmp = InternalKeyComparator(idb->GetOptions(cfh).comparator);
   ReadOptions read_options;
   Arena arena;
-  ScopedArenaIterator iter(
+  ScopedArenaPtr<InternalIterator> iter(
       idb->NewInternalIterator(read_options, &arena, kMaxSequenceNumber, cfh));
 
   if (!begin_key.empty()) {