ARROW-8113: [C++] Lighter weight variant<>

LICENSE.txt

@@ -680,34 +680,6 @@ DEALINGS IN THE SOFTWARE.
 
 --------------------------------------------------------------------------------
 
-The file cpp/src/arrow/vendored/variant.hpp has the following license
-
-Boost Software License - Version 1.0 - August 17th, 2003
-
-Permission is hereby granted, free of charge, to any person or organization
-obtaining a copy of the software and accompanying documentation covered by
-this license (the "Software") to use, reproduce, display, distribute,
-execute, and transmit the Software, and to prepare derivative works of the
-Software, and to permit third-parties to whom the Software is furnished to
-do so, all subject to the following:
-
-The copyright notices in the Software and this entire statement, including
-the above license grant, this restriction and the following disclaimer,
-must be included in all copies of the Software, in whole or in part, and
-all derivative works of the Software, unless such copies or derivative
-works are solely in the form of machine-executable object code generated by
-a source language processor.
-
-THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
-IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
-FITNESS FOR A PARTICULAR PURPOSE, TITLE AND NON-INFRINGEMENT. IN NO EVENT
-SHALL THE COPYRIGHT HOLDERS OR ANYONE DISTRIBUTING THE SOFTWARE BE LIABLE
-FOR ANY DAMAGES OR OTHER LIABILITY, WHETHER IN CONTRACT, TORT OR OTHERWISE,
-ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
-DEALINGS IN THE SOFTWARE.
-
---------------------------------------------------------------------------------
-
 The files in cpp/src/arrow/vendored/xxhash/ have the following license
 (BSD 2-Clause License)
 

cpp/src/arrow/compute/function_benchmark.cc

@@ -28,6 +28,7 @@
 namespace arrow {
 
 using internal::checked_cast;
+using internal::checked_pointer_cast;
 
 namespace compute {
 
@@ -43,7 +44,7 @@ inline ScalarVector ToScalars(std::shared_ptr<Array> arr) {
   return scalars;
 }
 
-void BM_CastDispatch(benchmark::State& state) {  // NOLINT non-const reference
+void BM_CastDispatch(benchmark::State& state) {
   // Repeatedly invoke a trivial Cast: the main cost should be dispatch
   random::RandomArrayGenerator rag(kSeed);
 
@@ -61,7 +62,7 @@ void BM_CastDispatch(benchmark::State& state) {  // NOLINT non-const reference
   state.SetItemsProcessed(state.iterations() * kScalarCount);
 }
 
-void BM_CastDispatchBaseline(benchmark::State& state) {  // NOLINT non-const reference
+void BM_CastDispatchBaseline(benchmark::State& state) {
   // Repeatedly invoke a trivial Cast with all dispatch outside the hot loop
   random::RandomArrayGenerator rag(kSeed);
 
@@ -94,7 +95,7 @@ void BM_CastDispatchBaseline(benchmark::State& state) {  // NOLINT non-const ref
   state.SetItemsProcessed(state.iterations() * kScalarCount);
 }
 
-void BM_AddDispatch(benchmark::State& state) {  // NOLINT non-const reference
+void BM_AddDispatch(benchmark::State& state) {
   ExecContext exec_context;
   KernelContext kernel_context(&exec_context);
 
@@ -109,9 +110,75 @@ void BM_AddDispatch(benchmark::State& state) {  // NOLINT non-const reference
   state.SetItemsProcessed(state.iterations());
 }
 
-BENCHMARK(BM_CastDispatch)->MinTime(1.0);
-BENCHMARK(BM_CastDispatchBaseline)->MinTime(1.0);
-BENCHMARK(BM_AddDispatch)->MinTime(1.0);
+static ScalarVector MakeScalarsForIsValid(int64_t nitems) {
+  std::vector<std::shared_ptr<Scalar>> scalars;
+  scalars.reserve(nitems);
+  for (int64_t i = 0; i < nitems; ++i) {
+    if (i & 0x10) {
+      scalars.emplace_back(MakeNullScalar(int64()));
+    } else {
+      scalars.emplace_back(*MakeScalar(int64(), i));
+    }
+  }
+  return scalars;
+}
+
+void BM_ExecuteScalarFunctionOnScalar(benchmark::State& state) {
+  // Execute a trivial function, with argument dispatch in the hot path
+  const int64_t N = 10000;
+
+  auto function = checked_pointer_cast<ScalarFunction>(
+      *GetFunctionRegistry()->GetFunction("is_valid"));
+  const auto scalars = MakeScalarsForIsValid(N);
+
+  ExecContext exec_context;
+  KernelContext kernel_context(&exec_context);
+
+  for (auto _ : state) {
+    int64_t total = 0;
+    for (const auto& scalar : scalars) {
+      const Datum result =
+          *function->Execute({Datum(scalar)}, function->default_options(), &exec_context);
+      total += result.scalar()->is_valid;
+    }
+    benchmark::DoNotOptimize(total);
+  }
+
+  state.SetItemsProcessed(state.iterations() * N);
+}
+
+void BM_ExecuteScalarKernelOnScalar(benchmark::State& state) {
+  // Execute a trivial function, with argument dispatch outside the hot path
+  const int64_t N = 10000;
+
+  auto function = *GetFunctionRegistry()->GetFunction("is_valid");
+  auto kernel = *function->DispatchExact({ValueDescr::Scalar(int64())});
+  const auto& exec = static_cast<const ScalarKernel&>(*kernel).exec;
+
+  const auto scalars = MakeScalarsForIsValid(N);
+
+  ExecContext exec_context;
+  KernelContext kernel_context(&exec_context);
+
+  for (auto _ : state) {
+    int64_t total = 0;
+    for (const auto& scalar : scalars) {
+      Datum result{MakeNullScalar(int64())};
+      exec(&kernel_context, ExecBatch{{scalar}, /*length=*/1}, &result);
+      ABORT_NOT_OK(kernel_context.status());
+      total += result.scalar()->is_valid;
+    }
+    benchmark::DoNotOptimize(total);
+  }
+
+  state.SetItemsProcessed(state.iterations() * N);
+}
+
+BENCHMARK(BM_CastDispatch);
+BENCHMARK(BM_CastDispatchBaseline);
+BENCHMARK(BM_AddDispatch);
+BENCHMARK(BM_ExecuteScalarFunctionOnScalar);
+BENCHMARK(BM_ExecuteScalarKernelOnScalar);
 
 }  // namespace compute
 }  // namespace arrow

cpp/src/arrow/dataset/filter.h

@@ -414,7 +414,7 @@ class ARROW_DS_EXPORT CastExpression final
   const std::shared_ptr<Expression>& like_expr() const;
 
  private:
-  util::variant<std::shared_ptr<DataType>, std::shared_ptr<Expression>> to_;
+  util::Variant<std::shared_ptr<DataType>, std::shared_ptr<Expression>> to_;
   CastOptions options_;
 };
 

cpp/src/arrow/datum.h

@@ -104,13 +104,13 @@ struct ARROW_EXPORT Datum {
   // current variant does not have a length.
   static constexpr int64_t kUnknownLength = -1;
 
-  util::variant<decltype(NULLPTR), std::shared_ptr<Scalar>, std::shared_ptr<ArrayData>,
+  util::Variant<decltype(NULLPTR), std::shared_ptr<Scalar>, std::shared_ptr<ArrayData>,
                 std::shared_ptr<ChunkedArray>, std::shared_ptr<RecordBatch>,
                 std::shared_ptr<Table>, std::vector<Datum>>
       value;
 
   /// \brief Empty datum, to be populated elsewhere
-  Datum() : value(NULLPTR) {}
+  Datum() = default;
 
   Datum(std::shared_ptr<Scalar> value)  // NOLINT implicit conversion
       : value(std::move(value)) {}

cpp/src/arrow/filesystem/mockfs.cc

@@ -50,7 +50,7 @@ struct File {
   std::string name;
   std::string data;
 
-  File(TimePoint mtime, const std::string& name) : mtime(mtime), name(name) {}
+  File(TimePoint mtime, std::string name) : mtime(mtime), name(std::move(name)) {}
 
   int64_t size() const { return static_cast<int64_t>(data.length()); }
 };
@@ -60,8 +60,18 @@ struct Directory {
   TimePoint mtime;
   std::map<std::string, std::unique_ptr<Entry>> entries;
 
-  Directory(const std::string& name, TimePoint mtime) : name(name), mtime(mtime) {}
-  Directory(Directory&&) = default;
+  Directory(std::string name, TimePoint mtime) : name(std::move(name)), mtime(mtime) {}
+  Directory(Directory&& other) noexcept
+      : name(std::move(other.name)),
+        mtime(other.mtime),
+        entries(std::move(other.entries)) {}
+
+  Directory& operator=(Directory&& other) noexcept {
+    name = std::move(other.name);
+    mtime = other.mtime;
+    entries = std::move(other.entries);
+    return *this;
+  }
 
   Entry* Find(const std::string& s) {
     auto it = entries.find(s);
@@ -90,11 +100,12 @@ struct Directory {
 };
 
 // A filesystem entry
-using EntryBase = util::variant<File, Directory>;
+using EntryBase = util::Variant<std::nullptr_t, File, Directory>;
 
 class Entry : public EntryBase {
  public:
   Entry(Entry&&) = default;
+  Entry& operator=(Entry&&) = default;
   explicit Entry(Directory&& v) : EntryBase(std::move(v)) {}
   explicit Entry(File&& v) : EntryBase(std::move(v)) {}
 
@@ -163,7 +174,7 @@ class MockFSOutputStream : public io::OutputStream {
  public:
   explicit MockFSOutputStream(File* file) : file_(file), closed_(false) {}
 
-  ~MockFSOutputStream() override {}
+  ~MockFSOutputStream() override = default;
 
   // Implement the OutputStream interface
   Status Close() override {
@@ -284,7 +295,7 @@ class MockFileSystem::Impl {
   }
 
   void GatherInfos(const FileSelector& select, const std::string& base_path,
-                   Directory& base_dir, int32_t nesting_depth,
+                   const Directory& base_dir, int32_t nesting_depth,
                    std::vector<FileInfo>* infos) {
     for (const auto& pair : base_dir.entries) {
       Entry* child = pair.second.get();
@@ -297,7 +308,7 @@ class MockFileSystem::Impl {
     }
   }
 
-  void DumpDirs(const std::string& prefix, Directory& dir,
+  void DumpDirs(const std::string& prefix, const Directory& dir,
                 std::vector<MockDirInfo>* out) {
     std::string path = prefix + dir.name;
     if (!path.empty()) {
@@ -312,7 +323,7 @@ class MockFileSystem::Impl {
     }
   }
 
-  void DumpFiles(const std::string& prefix, Directory& dir,
+  void DumpFiles(const std::string& prefix, const Directory& dir,
                  std::vector<MockFileInfo>* out) {
     std::string path = prefix + dir.name;
     if (!path.empty()) {
@@ -370,7 +381,7 @@ class MockFileSystem::Impl {
   }
 };
 
-MockFileSystem::~MockFileSystem() {}
+MockFileSystem::~MockFileSystem() = default;
 
 MockFileSystem::MockFileSystem(TimePoint current_time) {
   impl_ = std::unique_ptr<Impl>(new Impl(current_time));

cpp/src/arrow/flight/client.h

@@ -117,7 +117,7 @@ class ARROW_FLIGHT_EXPORT FlightClientOptions {
 
   /// \brief Generic connection options, passed to the underlying
   ///     transport; interpretation is implementation-dependent.
-  std::vector<std::pair<std::string, util::variant<int, std::string>>> generic_options;
+  std::vector<std::pair<std::string, util::Variant<int, std::string>>> generic_options;
 
   /// \brief Use TLS without validating the server certificate. Use with caution.
   bool disable_server_verification;

cpp/src/arrow/type.cc

@@ -1005,13 +1005,13 @@ FieldRef::FieldRef(FieldPath indices) : impl_(std::move(indices)) {
 void FieldRef::Flatten(std::vector<FieldRef> children) {
   // flatten children
   struct Visitor {
-    void operator()(std::string&& name) { *out++ = FieldRef(std::move(name)); }
+    void operator()(std::string* name) { *out++ = FieldRef(std::move(*name)); }
 
-    void operator()(FieldPath&& indices) { *out++ = FieldRef(std::move(indices)); }
+    void operator()(FieldPath* indices) { *out++ = FieldRef(std::move(*indices)); }
 
-    void operator()(std::vector<FieldRef>&& children) {
-      for (auto& child : children) {
-        util::visit(*this, std::move(child.impl_));
+    void operator()(std::vector<FieldRef>* children) {
+      for (auto& child : *children) {
+        util::visit(*this, &child.impl_);
       }
     }
 
@@ -1020,7 +1020,7 @@ void FieldRef::Flatten(std::vector<FieldRef> children) {
 
   std::vector<FieldRef> out;
   Visitor visitor{std::back_inserter(out)};
-  visitor(std::move(children));
+  visitor(&children);
 
   DCHECK(!out.empty());
   DCHECK(std::none_of(out.begin(), out.end(),

cpp/src/arrow/type.h

@@ -1596,7 +1596,7 @@ class ARROW_EXPORT FieldRef {
  private:
   void Flatten(std::vector<FieldRef> children);
 
-  util::variant<FieldPath, std::string, std::vector<FieldRef>> impl_;
+  util::Variant<FieldPath, std::string, std::vector<FieldRef>> impl_;
 
   ARROW_EXPORT friend void PrintTo(const FieldRef& ref, std::ostream* os);
 };

cpp/src/arrow/util/CMakeLists.txt

@@ -61,7 +61,8 @@ add_arrow_test(utility-test
                trie_test.cc
                uri_test.cc
                utf8_util_test.cc
-               value_parsing_test.cc)
+               value_parsing_test.cc
+               variant_test.cc)
 
 add_arrow_test(threading-utility-test
                SOURCES
@@ -81,3 +82,4 @@ add_arrow_benchmark(thread_pool_benchmark)
 add_arrow_benchmark(trie_benchmark)
 add_arrow_benchmark(utf8_util_benchmark)
 add_arrow_benchmark(value_parsing_benchmark)
+add_arrow_benchmark(variant_benchmark)

cpp/src/arrow/util/type_traits.h

@@ -17,6 +17,7 @@
 
 #pragma once
 
+#include <cstdint>
 #include <type_traits>
 
 namespace arrow {
@@ -41,5 +42,45 @@ template <typename T>
 struct is_null_pointer : std::is_same<std::nullptr_t, typename std::remove_cv<T>::type> {
 };
 
+#ifdef __GLIBCXX__
+
+// A aligned_union backport, because old libstdc++ versions don't include it.
+
+constexpr std::size_t max_size(std::size_t a, std::size_t b) { return (a > b) ? a : b; }
+
+template <typename...>
+struct max_size_traits;
+
+template <typename H, typename... T>
+struct max_size_traits<H, T...> {
+  static constexpr std::size_t max_sizeof() {
+    return max_size(sizeof(H), max_size_traits<T...>::max_sizeof());
+  }
+  static constexpr std::size_t max_alignof() {
+    return max_size(alignof(H), max_size_traits<T...>::max_alignof());
+  }
+};
+
+template <>
+struct max_size_traits<> {
+  static constexpr std::size_t max_sizeof() { return 0; }
+  static constexpr std::size_t max_alignof() { return 0; }
+};
+
+template <std::size_t Len, typename... T>
+struct aligned_union {
+  static constexpr std::size_t alignment_value = max_size_traits<T...>::max_alignof();
+  static constexpr std::size_t size_value =
+      max_size(Len, max_size_traits<T...>::max_sizeof());
+  using type = typename std::aligned_storage<size_value, alignment_value>::type;
+};
+
+#else
+
+template <std::size_t Len, typename... T>
+using aligned_union = std::aligned_union<Len, T...>;
+
+#endif
+
 }  // namespace internal
 }  // namespace arrow