kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a...@apache.org
Subject [kudu] 01/07: env: remove kudu::Bind usage from Walk
Date Mon, 30 Mar 2020 18:56:35 GMT
This is an automated email from the ASF dual-hosted git repository.

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

commit 08464561d95651470957ddeceb94a1ad3170df3d
Author: Adar Dembo <adar@cloudera.com>
AuthorDate: Fri Mar 27 23:49:07 2020 -0700

    env: remove kudu::Bind usage from Walk
    
    This isn't as elegant as the other patches in this series due to the number
    of parameters in WalkCallback. C++14 allows the use of 'auto' for parameter
    types which can shorten them somewhat.
    
    Change-Id: I080809aef7e2f0ac3c240d4ca6909951b9615bb0
    Reviewed-on: http://gerrit.cloudera.org:8080/15575
    Tested-by: Kudu Jenkins
    Reviewed-by: Alexey Serbin <aserbin@cloudera.com>
    Reviewed-by: Andrew Wong <awong@cloudera.com>
---
 src/kudu/fs/block_manager-test.cc |  9 +++++----
 src/kudu/fs/file_block_manager.cc | 14 +++++++++-----
 src/kudu/util/env-test.cc         | 18 ++++++++++++------
 src/kudu/util/env.h               | 10 +++-------
 src/kudu/util/env_posix.cc        | 22 +++++++++++++---------
 src/kudu/util/env_util.cc         |  8 ++++++--
 src/kudu/util/env_util.h          |  5 +----
 src/kudu/util/metrics.h           |  2 +-
 8 files changed, 50 insertions(+), 38 deletions(-)

diff --git a/src/kudu/fs/block_manager-test.cc b/src/kudu/fs/block_manager-test.cc
index 88143d3..1aca697 100644
--- a/src/kudu/fs/block_manager-test.cc
+++ b/src/kudu/fs/block_manager-test.cc
@@ -43,7 +43,6 @@
 #include "kudu/fs/fs_report.h"
 #include "kudu/fs/log_block_manager.h"
 #include "kudu/gutil/basictypes.h"
-#include "kudu/gutil/bind.h"
 #include "kudu/gutil/casts.h"
 #include "kudu/gutil/map-util.h"
 #include "kudu/gutil/ref_counted.h"
@@ -224,9 +223,11 @@ class BlockManagerTest : public KuduTest {
   // hierarchy, ignoring '.', '..', and file 'kInstanceMetadataFileName'.
   Status CountFiles(const string& root, int* num_files) {
     *num_files = 0;
-    RETURN_NOT_OK(env_->Walk(root, Env::PRE_ORDER,
-                             Bind(BlockManagerTest::CountFilesCb, num_files)));
-    return Status::OK();
+    return env_->Walk(
+        root, Env::PRE_ORDER,
+        [num_files](Env::FileType type, const string& dirname, const string& basename)
{
+          return CountFilesCb(num_files, type, dirname, basename);
+        });
   }
 
   // Keep an internal copy of the data dir group to act as metadata.
diff --git a/src/kudu/fs/file_block_manager.cc b/src/kudu/fs/file_block_manager.cc
index 09b2a99..ae8f1bc 100644
--- a/src/kudu/fs/file_block_manager.cc
+++ b/src/kudu/fs/file_block_manager.cc
@@ -18,6 +18,7 @@
 #include "kudu/fs/file_block_manager.h"
 
 #include <cstddef>
+#include <functional>
 #include <memory>
 #include <mutex>
 #include <numeric>
@@ -921,11 +922,14 @@ Status GetAllBlockIdsForDataDirCb(Dir* dd,
 }
 
 void GetAllBlockIdsForDir(Env* env,
-                              Dir* dd,
-                              vector<BlockId>* block_ids,
-                              Status* status) {
-  *status = env->Walk(dd->dir(), Env::PRE_ORDER,
-                      Bind(&GetAllBlockIdsForDataDirCb, dd, block_ids));
+                          Dir* dd,
+                          vector<BlockId>* block_ids,
+                          Status* status) {
+  *status = env->Walk(
+      dd->dir(), Env::PRE_ORDER,
+      [dd, block_ids](Env::FileType type, const string& dirname, const string& basename)
{
+        return GetAllBlockIdsForDataDirCb(dd, block_ids, type, dirname, basename);
+      });
 }
 
 } // anonymous namespace
diff --git a/src/kudu/util/env-test.cc b/src/kudu/util/env-test.cc
index 6dda618..3eb4253 100644
--- a/src/kudu/util/env-test.cc
+++ b/src/kudu/util/env-test.cc
@@ -52,7 +52,6 @@
 #include <glog/stl_logging.h> // IWYU pragma: keep
 #include <gtest/gtest.h>
 
-#include "kudu/gutil/bind.h"
 #include "kudu/gutil/macros.h"
 #include "kudu/gutil/map-util.h"
 #include "kudu/gutil/port.h"
@@ -752,13 +751,17 @@ TEST_F(TestEnv, TestWalk) {
 
   // Do the walk.
   unordered_set<string> actual;
-  ASSERT_OK(env_->Walk(root, Env::PRE_ORDER, Bind(&TestWalkCb, &actual)));
+  ASSERT_OK(env_->Walk(
+      root, Env::PRE_ORDER,
+      [&actual](Env::FileType type, const string& dirname, const string& basename)
{
+        return TestWalkCb(&actual, type, dirname, basename);
+      }));
   ASSERT_EQ(expected, actual);
 }
 
 TEST_F(TestEnv, TestWalkNonExistentPath) {
   // A walk on a non-existent path should fail.
-  Status s = env_->Walk("/not/a/real/path", Env::PRE_ORDER, Bind(&NoopTestWalkCb));
+  Status s = env_->Walk("/not/a/real/path", Env::PRE_ORDER, &NoopTestWalkCb);
   ASSERT_TRUE(s.IsIOError());
   ASSERT_STR_CONTAINS(s.ToString(), "One or more errors occurred");
 }
@@ -777,7 +780,7 @@ TEST_F(TestEnv, TestWalkBadPermissions) {
 
   // A walk on a directory without execute permission should fail,
   // unless the calling process has super-user's effective ID.
-  Status s = env_->Walk(kTestPath, Env::PRE_ORDER, Bind(&NoopTestWalkCb));
+  Status s = env_->Walk(kTestPath, Env::PRE_ORDER, &NoopTestWalkCb);
   if (geteuid() == 0) {
     ASSERT_TRUE(s.ok()) << s.ToString();
   } else {
@@ -801,8 +804,11 @@ TEST_F(TestEnv, TestWalkCbReturnsError) {
   unique_ptr<WritableFile> writer;
   ASSERT_OK(env_->NewWritableFile(JoinPathSegments(new_dir, new_file), &writer));
   int num_calls = 0;
-  ASSERT_TRUE(env_->Walk(new_dir, Env::PRE_ORDER,
-                         Bind(&TestWalkErrorCb, &num_calls)).IsIOError());
+  ASSERT_TRUE(env_->Walk(
+      new_dir, Env::PRE_ORDER,
+      [&num_calls](Env::FileType type, const string& dirname, const string& basename)
{
+        return TestWalkErrorCb(&num_calls, type, dirname, basename);
+      }).IsIOError());
 
   // Once for the directory and once for the file inside it.
   ASSERT_EQ(2, num_calls);
diff --git a/src/kudu/util/env.h b/src/kudu/util/env.h
index 9a2d7bc..7c57bb3 100644
--- a/src/kudu/util/env.h
+++ b/src/kudu/util/env.h
@@ -9,19 +9,17 @@
 //
 // All Env implementations are safe for concurrent access from
 // multiple threads without any external synchronization.
-
-#ifndef STORAGE_LEVELDB_INCLUDE_ENV_H_
-#define STORAGE_LEVELDB_INCLUDE_ENV_H_
+#pragma once
 
 #include <cstddef>
 #include <cstdint>
+#include <functional>
 #include <iosfwd>
 #include <map>
 #include <memory>
 #include <string>
 #include <vector>
 
-#include "kudu/gutil/callback_forward.h"
 #include "kudu/gutil/macros.h"
 #include "kudu/util/status.h"
 
@@ -278,7 +276,7 @@ class Env {
   //
   // Returning an error won't halt the walk, but it will cause it to return
   // with an error status when it's done.
-  typedef Callback<Status(FileType, const std::string&, const std::string&)>
WalkCallback;
+  typedef std::function<Status(FileType, const std::string&, const std::string&)>
WalkCallback;
 
   // Whether to walk directories in pre-order or post-order.
   enum DirectoryOrder {
@@ -702,5 +700,3 @@ extern Status ReadFileToString(Env* env, const std::string& fname,
 std::ostream& operator<<(std::ostream& o, Env::ResourceLimitType t);
 
 }  // namespace kudu
-
-#endif  // STORAGE_LEVELDB_INCLUDE_ENV_H_
diff --git a/src/kudu/util/env_posix.cc b/src/kudu/util/env_posix.cc
index 3590fd1..586b333 100644
--- a/src/kudu/util/env_posix.cc
+++ b/src/kudu/util/env_posix.cc
@@ -26,6 +26,7 @@
 #include <cstdlib>
 #include <cstring>
 #include <ctime>
+#include <functional>
 #include <map>
 #include <memory>
 #include <numeric>
@@ -39,8 +40,6 @@
 
 #include "kudu/gutil/atomicops.h"
 #include "kudu/gutil/basictypes.h"
-#include "kudu/gutil/bind.h"
-#include "kudu/gutil/bind_helpers.h"
 #include "kudu/gutil/macros.h"
 #include "kudu/gutil/map-util.h"
 #include "kudu/gutil/once.h"
@@ -1332,8 +1331,11 @@ class PosixEnv : public Env {
   }
 
   virtual Status DeleteRecursively(const string &name) OVERRIDE {
-    return Walk(name, POST_ORDER, Bind(&PosixEnv::DeleteRecursivelyCb,
-                                       Unretained(this)));
+    return Walk(
+        name, POST_ORDER,
+        [this](FileType type, const string& dirname, const string& basename) ->
Status {
+          return this->DeleteRecursivelyCb(type, dirname, basename);
+        });
   }
 
   virtual Status GetFileSize(const string& fname, uint64_t* size) OVERRIDE {
@@ -1373,9 +1375,11 @@ class PosixEnv : public Env {
                                               uint64_t* bytes_used) OVERRIDE {
     TRACE_EVENT1("io", "PosixEnv::GetFileSizeOnDiskRecursively", "path", root);
     uint64_t total = 0;
-    RETURN_NOT_OK(Walk(root, Env::PRE_ORDER,
-                       Bind(&PosixEnv::GetFileSizeOnDiskRecursivelyCb,
-                            Unretained(this), &total)));
+    RETURN_NOT_OK(Walk(
+        root, PRE_ORDER,
+        [this, &total](FileType type, const string& dirname, const string& basename)
-> Status {
+          return this->GetFileSizeOnDiskRecursivelyCb(&total, type, dirname, basename);
+        }));
     *bytes_used = total;
     return Status::OK();
   }
@@ -1625,7 +1629,7 @@ class PosixEnv : public Env {
           break;
       }
       if (doCb) {
-        if (!cb.Run(type, DirName(ent->fts_path), ent->fts_name).ok()) {
+        if (!cb(type, DirName(ent->fts_path), ent->fts_name).ok()) {
           had_errors = true;
         }
       }
@@ -1891,7 +1895,7 @@ class PosixEnv : public Env {
   }
 
   Status GetFileSizeOnDiskRecursivelyCb(uint64_t* bytes_used,
-                                        Env::FileType type,
+                                        FileType type,
                                         const string& dirname,
                                         const string& basename) {
     uint64_t file_bytes_used = 0;
diff --git a/src/kudu/util/env_util.cc b/src/kudu/util/env_util.cc
index 6fb8b8a..41f0ad1 100644
--- a/src/kudu/util/env_util.cc
+++ b/src/kudu/util/env_util.cc
@@ -23,6 +23,7 @@
 #include <cerrno>
 #include <cstdint>
 #include <ctime>
+#include <functional>
 #include <memory>
 #include <string>
 #include <unordered_set>
@@ -32,7 +33,6 @@
 #include <gflags/gflags.h>
 #include <glog/logging.h>
 
-#include "kudu/gutil/bind.h"
 #include "kudu/gutil/macros.h"
 #include "kudu/gutil/port.h"
 #include "kudu/gutil/strings/split.h"
@@ -309,7 +309,11 @@ static Status DeleteTmpFilesRecursivelyCb(Env* env,
 }
 
 Status DeleteTmpFilesRecursively(Env* env, const string& path) {
-  return env->Walk(path, Env::PRE_ORDER, Bind(&DeleteTmpFilesRecursivelyCb, env));
+  return env->Walk(
+      path, Env::PRE_ORDER,
+      [env](Env::FileType type, const string& dirname, const string& basename) {
+        return DeleteTmpFilesRecursivelyCb(env, type, dirname, basename);
+      });
 }
 
 Status IsDirectoryEmpty(Env* env, const string& path, bool* is_empty) {
diff --git a/src/kudu/util/env_util.h b/src/kudu/util/env_util.h
index bba1e92..00007d6 100644
--- a/src/kudu/util/env_util.h
+++ b/src/kudu/util/env_util.h
@@ -14,8 +14,7 @@
 // KIND, either express or implied.  See the License for the
 // specific language governing permissions and limitations
 // under the License.
-#ifndef KUDU_UTIL_ENV_UTIL_H
-#define KUDU_UTIL_ENV_UTIL_H
+#pragma once
 
 #include <cstddef>
 #include <cstdint>
@@ -111,5 +110,3 @@ Status ListFilesInDir(Env* env,
 
 } // namespace env_util
 } // namespace kudu
-
-#endif
diff --git a/src/kudu/util/metrics.h b/src/kudu/util/metrics.h
index 7a1ed07..ab31113 100644
--- a/src/kudu/util/metrics.h
+++ b/src/kudu/util/metrics.h
@@ -1169,7 +1169,7 @@ class AtomicGauge : public Gauge {
 //  public:
 //   MyClassWithMetrics(const scoped_refptr<MetricEntity>& entity) {
 //     METRIC_my_metric.InstantiateFunctionGauge(entity,
-//       Bind(&MyClassWithMetrics::ComputeMyMetric, Unretained(this)))
+//       [this]() { return this->ComputeMyMetric(); })
 //       ->AutoDetach(&metric_detacher_);
 //   }
 //   ~MyClassWithMetrics() {


Mime
View raw message