kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From granthe...@apache.org
Subject [kudu] 01/03: [tserver] replace gscoped_ptr with unique_ptr in Scanner
Date Thu, 16 Jan 2020 22:46:51 GMT
This is an automated email from the ASF dual-hosted git repository.

granthenke pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit c8384a04029c3e1b4ce23888355a850721d0f8e7
Author: Alexey Serbin <alexey@apache.org>
AuthorDate: Wed Jan 15 19:32:42 2020 -0800

    [tserver] replace gscoped_ptr with unique_ptr in Scanner
    
    This is a small clean up to replace gscoped_ptr with std::unique_ptr
    and avoid extra call to operator new.  I also moved the
    Scanner::IsInitialized() method into the private section.
    
    Change-Id: Ieac533565f96773cc450de521703c21534020596
    Reviewed-on: http://gerrit.cloudera.org:8080/15048
    Reviewed-by: Adar Dembo <adar@cloudera.com>
    Tested-by: Alexey Serbin <aserbin@cloudera.com>
---
 src/kudu/tserver/scanners.cc       |  4 ++--
 src/kudu/tserver/scanners.h        | 32 +++++++++++++-------------
 src/kudu/tserver/tablet_service.cc | 47 +++++++++++++++++---------------------
 3 files changed, 39 insertions(+), 44 deletions(-)

diff --git a/src/kudu/tserver/scanners.cc b/src/kudu/tserver/scanners.cc
index 77b851b..418bbdc 100644
--- a/src/kudu/tserver/scanners.cc
+++ b/src/kudu/tserver/scanners.cc
@@ -359,11 +359,11 @@ void Scanner::AddTimings(const CpuTimes& elapsed) {
 }
 
 void Scanner::Init(unique_ptr<RowwiseIterator> iter,
-                   gscoped_ptr<ScanSpec> spec) {
+                   unique_ptr<ScanSpec> spec) {
   std::lock_guard<simple_spinlock> l(lock_);
   CHECK(!iter_) << "Already initialized";
   iter_ = std::move(iter);
-  spec_.reset(spec.release());
+  spec_ = std::move(spec);
 }
 
 const ScanSpec& Scanner::spec() const {
diff --git a/src/kudu/tserver/scanners.h b/src/kudu/tserver/scanners.h
index f03e8ff..15eb86c 100644
--- a/src/kudu/tserver/scanners.h
+++ b/src/kudu/tserver/scanners.h
@@ -30,6 +30,7 @@
 
 #include "kudu/common/iterator_stats.h"
 #include "kudu/common/scan_spec.h"
+#include "kudu/common/schema.h"
 #include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/gutil/macros.h"
 #include "kudu/gutil/ref_counted.h"
@@ -50,7 +51,6 @@
 namespace kudu {
 
 class RowwiseIterator;
-class Schema;
 class Status;
 class Thread;
 
@@ -203,15 +203,7 @@ class Scanner {
   // Attach an actual iterator and a ScanSpec to this Scanner.
   // Takes ownership of 'iter' and 'spec'.
   void Init(std::unique_ptr<RowwiseIterator> iter,
-            gscoped_ptr<ScanSpec> spec);
-
-  // Return true if the scanner has been initialized (i.e has an iterator).
-  // Once a Scanner is initialized, it is safe to assume that iter() and spec()
-  // return non-NULL for the lifetime of the Scanner object.
-  bool IsInitialized() const {
-    std::lock_guard<simple_spinlock> l(lock_);
-    return iter_ != NULL;
-  }
+            std::unique_ptr<ScanSpec> spec);
 
   RowwiseIterator* iter() {
     return DCHECK_NOTNULL(iter_.get());
@@ -282,8 +274,8 @@ class Scanner {
   // projection is a subset of the iterator's schema -- the iterator's
   // schema needs to include all columns that have predicates, whereas
   // the client may not want to project all of them.
-  void set_client_projection_schema(gscoped_ptr<Schema> client_projection_schema) {
-    client_projection_schema_.swap(client_projection_schema);
+  void set_client_projection_schema(std::unique_ptr<Schema> client_projection_schema)
{
+    client_projection_schema_ = std::move(client_projection_schema);
   }
 
   // Returns request's projection schema if it differs from the schema
@@ -332,6 +324,14 @@ class Scanner {
 
   static const std::string kNullTabletId;
 
+  // Return true if the scanner has been initialized (i.e has an iterator).
+  // Once a Scanner is initialized, it is safe to assume that iter() and spec()
+  // return non-NULL for the lifetime of the Scanner object.
+  bool IsInitialized() const {
+    std::lock_guard<simple_spinlock> l(lock_);
+    return iter_ != nullptr;
+  }
+
   // The unique ID of this scanner.
   const std::string id_;
 
@@ -364,13 +364,13 @@ class Scanner {
   IteratorStats already_reported_stats_;
 
   // The spec used by 'iter_'
-  gscoped_ptr<ScanSpec> spec_;
+  std::unique_ptr<ScanSpec> spec_;
+
+  std::unique_ptr<RowwiseIterator> iter_;
 
   // Stores the request's projection schema, if it differs from the
   // schema used by the iterator.
-  gscoped_ptr<Schema> client_projection_schema_;
-
-  std::unique_ptr<RowwiseIterator> iter_;
+  std::unique_ptr<Schema> client_projection_schema_;
 
   AutoReleasePool autorelease_pool_;
 
diff --git a/src/kudu/tserver/tablet_service.cc b/src/kudu/tserver/tablet_service.cc
index f642a7e..bde51f5 100644
--- a/src/kudu/tserver/tablet_service.cc
+++ b/src/kudu/tserver/tablet_service.cc
@@ -25,9 +25,7 @@
 #include <numeric>
 #include <ostream>
 #include <string>
-#include <type_traits>
 #include <unordered_set>
-#include <utility>
 #include <vector>
 
 #include <boost/optional/optional.hpp>
@@ -91,7 +89,6 @@
 #include "kudu/tserver/tserver_admin.pb.h"
 #include "kudu/tserver/tserver_service.pb.h"
 #include "kudu/util/auto_release_pool.h"
-#include "kudu/util/bitset.h"
 #include "kudu/util/crc.h"
 #include "kudu/util/debug/trace_event.h"
 #include "kudu/util/faststring.h"
@@ -2160,16 +2157,15 @@ static Status DecodeEncodedKeyRange(const NewScanRequestPB& scan_pb,
 
 static Status SetupScanSpec(const NewScanRequestPB& scan_pb,
                             const Schema& tablet_schema,
-                            gscoped_ptr<ScanSpec>* spec,
-                            const SharedScanner& scanner) {
-  gscoped_ptr<ScanSpec> ret(new ScanSpec);
-  ret->set_cache_blocks(scan_pb.cache_blocks());
+                            const SharedScanner& scanner,
+                            ScanSpec* spec) {
+  spec->set_cache_blocks(scan_pb.cache_blocks());
 
   // First the column predicates.
   for (const ColumnPredicatePB& pred_pb : scan_pb.column_predicates()) {
     boost::optional<ColumnPredicate> predicate;
     RETURN_NOT_OK(ColumnPredicateFromPB(tablet_schema, scanner->arena(), pred_pb, &predicate));
-    ret->AddPredicate(std::move(*predicate));
+    spec->AddPredicate(std::move(*predicate));
   }
 
   // Then the column range predicates.
@@ -2205,19 +2201,18 @@ static Status SetupScanSpec(const NewScanRequestPB& scan_pb,
     if (pred) {
       VLOG(3) << Substitute("Parsed predicate $0 from $1",
                             pred->ToString(), SecureShortDebugString(scan_pb));
-      ret->AddPredicate(*pred);
+      spec->AddPredicate(*pred);
     }
   }
 
   // Then any encoded key range predicates.
-  RETURN_NOT_OK(DecodeEncodedKeyRange(scan_pb, tablet_schema, scanner, ret.get()));
+  RETURN_NOT_OK(DecodeEncodedKeyRange(scan_pb, tablet_schema, scanner, spec));
 
   // If the scanner has a limit, set it now.
   if (scan_pb.has_limit()) {
-    ret->set_limit(scan_pb.limit());
+    spec->set_limit(scan_pb.limit());
   }
 
-  spec->swap(ret);
   return Status::OK();
 }
 
@@ -2325,17 +2320,16 @@ Status TabletServiceImpl::HandleNewScanRequest(TabletReplica* replica,
     }
   }
 
-  gscoped_ptr<ScanSpec> spec(new ScanSpec);
-
-  s = SetupScanSpec(scan_pb, tablet_schema, &spec, scanner);
+  ScanSpec spec;
+  s = SetupScanSpec(scan_pb, tablet_schema, scanner, &spec);
   if (PREDICT_FALSE(!s.ok())) {
     *error_code = TabletServerErrorPB::INVALID_SCAN_SPEC;
     return s;
   }
 
-  VLOG(3) << "Before optimizing scan spec: " << spec->ToString(tablet_schema);
-  spec->OptimizeScan(tablet_schema, scanner->arena(), scanner->autorelease_pool(),
true);
-  VLOG(3) << "After optimizing scan spec: " << spec->ToString(tablet_schema);
+  VLOG(3) << "Before optimizing scan spec: " << spec.ToString(tablet_schema);
+  spec.OptimizeScan(tablet_schema, scanner->arena(), scanner->autorelease_pool(), true);
+  VLOG(3) << "After optimizing scan spec: " << spec.ToString(tablet_schema);
 
   // Missing columns will contain the columns that are not mentioned in the
   // client projection but are actually needed for the scan, such as columns
@@ -2343,16 +2337,18 @@ Status TabletServiceImpl::HandleNewScanRequest(TabletReplica* replica,
   //
   // NOTE: We should build the missing column after optimizing scan which will
   // remove unnecessary predicates.
-  vector<ColumnSchema> missing_cols = spec->GetMissingColumns(projection);
-  if (spec->CanShortCircuit()) {
+  vector<ColumnSchema> missing_cols = spec.GetMissingColumns(projection);
+  if (spec.CanShortCircuit()) {
     VLOG(1) << "short-circuiting without creating a server-side scanner.";
     *has_more_results = false;
     return Status::OK();
   }
 
   // Store the original projection.
-  gscoped_ptr<Schema> orig_projection(new Schema(projection));
-  scanner->set_client_projection_schema(std::move(orig_projection));
+  {
+    unique_ptr<Schema> orig_projection(new Schema(projection));
+    scanner->set_client_projection_schema(std::move(orig_projection));
+  }
 
   // Build a new projection with the projection columns and the missing columns,
   // annotating each column as a key column appropriately.
@@ -2395,8 +2391,6 @@ Status TabletServiceImpl::HandleNewScanRequest(TabletReplica* replica,
   projection = projection_builder.BuildWithoutIds();
   VLOG(3) << "Scan projection: " << projection.ToString(Schema::BASE_INFO);
 
-  unique_ptr<RowwiseIterator> iter;
-
   // It's important to keep the reference to the tablet for the case when the
   // tablet replica's shutdown is run concurrently with the code below.
   shared_ptr<Tablet> tablet;
@@ -2413,6 +2407,7 @@ Status TabletServiceImpl::HandleNewScanRequest(TabletReplica* replica,
     return s;
   }
 
+  unique_ptr<RowwiseIterator> iter;
   boost::optional<Timestamp> snap_start_timestamp;
 
   {
@@ -2448,11 +2443,11 @@ Status TabletServiceImpl::HandleNewScanRequest(TabletReplica* replica,
   // This copy will be given to the Scanner so it can report its predicates to
   // /scans. The copy is necessary because the original spec will be modified
   // as its predicates are pushed into lower-level iterators.
-  gscoped_ptr<ScanSpec> orig_spec(new ScanSpec(*spec));
+  unique_ptr<ScanSpec> orig_spec(new ScanSpec(spec));
 
   if (PREDICT_TRUE(s.ok())) {
     TRACE_EVENT0("tserver", "iter->Init");
-    s = iter->Init(spec.get());
+    s = iter->Init(&spec);
     if (PREDICT_FALSE(s.IsInvalidArgument())) {
       // Tablet::Iterator::Init() returns InvalidArgument when an invalid
       // projection is specified.


Mime
View raw message