Move MeasureTime to common namespace (facebookincubator#9567)

Summary:
Pull Request resolved: facebookincubator#9567

Make it more general.

Reviewed By: kgpai, Sullivan-Patrick

Differential Revision: D56436083

fbshipit-source-id: 635105e87157345ecd994ff934bee00d8ff18148

Author: Daniel Munoz

velox/dwio/common/MeasureTime.h

@@ -0,0 +1,67 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include <chrono>
+#include <functional>
+#include <optional>
+
+namespace facebook {
+namespace velox {
+namespace dwio {
+namespace common {
+
+class MeasureTime {
+ public:
+  explicit MeasureTime(
+      const std::function<void(std::chrono::high_resolution_clock::duration)>&
+          callback)
+      : callback_{callback},
+        startTime_{std::chrono::high_resolution_clock::now()} {}
+
+  MeasureTime(const MeasureTime&) = delete;
+  MeasureTime(MeasureTime&&) = delete;
+  MeasureTime& operator=(const MeasureTime&) = delete;
+  MeasureTime& operator=(MeasureTime&& other) = delete;
+
+  ~MeasureTime() {
+    callback_(std::chrono::high_resolution_clock::now() - startTime_);
+  }
+
+ private:
+  const std::function<void(std::chrono::high_resolution_clock::duration)>&
+      callback_;
+  const std::chrono::time_point<std::chrono::high_resolution_clock> startTime_;
+};
+
+// Make sure you don't pass a lambda to this function, because that will cause a
+// std::function to be created on the fly (implicitly), and when we return from
+// this function that std::function won't exist anymore. So when MeasureTime is
+// destroyed, it will try to access a non-existing std::function.
+inline std::optional<MeasureTime> measureTimeIfCallback(
+    const std::function<void(std::chrono::high_resolution_clock::duration)>&
+        callback) {
+  if (callback) {
+    return std::make_optional<MeasureTime>(callback);
+  }
+  return std::nullopt;
+}
+
+} // namespace common
+} // namespace dwio
+} // namespace velox
+} // namespace facebook

velox/dwio/common/OnDemandUnitLoader.cpp

@@ -17,9 +17,10 @@
 #include "velox/dwio/common/OnDemandUnitLoader.h"
 
 #include "velox/common/base/Exceptions.h"
+#include "velox/dwio/common/MeasureTime.h"
 #include "velox/dwio/common/UnitLoaderTools.h"
 
-using facebook::velox::dwio::common::unit_loader_tools::measureWithCallback;
+using facebook::velox::dwio::common::measureTimeIfCallback;
 
 namespace facebook::velox::dwio::common {
 
@@ -49,7 +50,7 @@ class OnDemandUnitLoader : public UnitLoader {
     }
 
     {
-      auto measure = measureWithCallback(blockedOnIoCallback_);
+      auto measure = measureTimeIfCallback(blockedOnIoCallback_);
       loadUnits_[unit]->load();
     }
     loadedUnit_ = unit;

velox/dwio/common/UnitLoaderTools.h

@@ -28,38 +28,6 @@
 
 namespace facebook::velox::dwio::common::unit_loader_tools {
 
-class Measure {
- public:
-  explicit Measure(
-      const std::function<void(std::chrono::high_resolution_clock::duration)>&
-          callback)
-      : callback_{callback},
-        startTime_{std::chrono::high_resolution_clock::now()} {}
-
-  Measure(const Measure&) = delete;
-  Measure(Measure&&) = delete;
-  Measure& operator=(const Measure&) = delete;
-  Measure& operator=(Measure&& other) = delete;
-
-  ~Measure() {
-    callback_(std::chrono::high_resolution_clock::now() - startTime_);
-  }
-
- private:
-  const std::function<void(std::chrono::high_resolution_clock::duration)>&
-      callback_;
-  const std::chrono::time_point<std::chrono::high_resolution_clock> startTime_;
-};
-
-inline std::optional<Measure> measureWithCallback(
-    const std::function<void(std::chrono::high_resolution_clock::duration)>&
-        callback) {
-  if (callback) {
-    return std::make_optional<Measure>(callback);
-  }
-  return std::nullopt;
-}
-
 // This class can create many callbacks that can be distributed to unit loader
 // factories. Only when the last created callback is activated, this class will
 // emit the original callback.

velox/dwio/common/tests/CMakeLists.txt

@@ -27,6 +27,7 @@ add_executable(
   LocalFileSinkTest.cpp
   MemorySinkTest.cpp
   LoggedExceptionTest.cpp
+  MeasureTimeTests.cpp
   ParallelForTest.cpp
   RangeTests.cpp
   ReadFileInputStreamTests.cpp

velox/dwio/common/tests/MeasureTimeTests.cpp

@@ -0,0 +1,46 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <gtest/gtest.h>
+
+#include "velox/dwio/common/MeasureTime.h"
+
+using namespace ::testing;
+using namespace ::facebook::velox::dwio::common;
+
+TEST(MeasureTimeTests, DoesntCreateMeasureIfNoCallback) {
+  EXPECT_FALSE(measureTimeIfCallback(nullptr).has_value());
+}
+
+TEST(MeasureTimeTests, CreatesMeasureIfCallback) {
+  auto callback =
+      std::function<void(std::chrono::high_resolution_clock::duration)>(
+          [](const auto&) {});
+  EXPECT_TRUE(measureTimeIfCallback(callback).has_value());
+}
+
+TEST(MeasureTimeTests, MeasuresTime) {
+  bool measured{false};
+  {
+    auto callback =
+        std::function<void(std::chrono::high_resolution_clock::duration)>(
+            [&measured](const auto&) { measured = true; });
+    auto measure = measureTimeIfCallback(callback);
+    EXPECT_TRUE(measure.has_value());
+    EXPECT_FALSE(measured);
+  }
+  EXPECT_TRUE(measured);
+}