arrow-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From w...@apache.org
Subject arrow git commit: ARROW-1151: [C++] Add branch prediction to RETURN_NOT_OK
Date Mon, 26 Jun 2017 18:02:04 GMT
Repository: arrow
Updated Branches:
  refs/heads/master 5de6eb5ff -> ec6e183ee


ARROW-1151: [C++] Add branch prediction to RETURN_NOT_OK

Also added some missing status checks to builder-benchmark

Author: Wes McKinney <wes.mckinney@twosigma.com>

Closes #782 from wesm/ARROW-1151 and squashes the following commits:

9b488a0e [Wes McKinney] Try to fix snappy warning
06276119 [Wes McKinney] Restore check macros used in libplasma
83b3f36d [Wes McKinney] Add branch prediction to RETURN_NOT_OK


Project: http://git-wip-us.apache.org/repos/asf/arrow/repo
Commit: http://git-wip-us.apache.org/repos/asf/arrow/commit/ec6e183e
Tree: http://git-wip-us.apache.org/repos/asf/arrow/tree/ec6e183e
Diff: http://git-wip-us.apache.org/repos/asf/arrow/diff/ec6e183e

Branch: refs/heads/master
Commit: ec6e183ee31411a18c531507216c21cc80060c8b
Parents: 5de6eb5
Author: Wes McKinney <wes.mckinney@twosigma.com>
Authored: Mon Jun 26 14:01:59 2017 -0400
Committer: Wes McKinney <wes.mckinney@twosigma.com>
Committed: Mon Jun 26 14:01:59 2017 -0400

----------------------------------------------------------------------
 LICENSE.txt                        | 34 ++++++++++++++++++++++++++++
 cpp/src/arrow/builder-benchmark.cc | 40 +++++++++++++++++----------------
 cpp/src/arrow/status.h             | 36 +++++++++--------------------
 cpp/src/arrow/util/compression.cc  |  5 +++++
 cpp/src/arrow/util/macros.h        | 14 ++++++++++++
 5 files changed, 84 insertions(+), 45 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/arrow/blob/ec6e183e/LICENSE.txt
----------------------------------------------------------------------
diff --git a/LICENSE.txt b/LICENSE.txt
index 7000733..95c506f 100644
--- a/LICENSE.txt
+++ b/LICENSE.txt
@@ -296,3 +296,37 @@ You can contact the author at :
 - xxHash source repository : https://github.com/Cyan4973/xxHash
 
 --------------------------------------------------------------------------------
+
+src/arrow/util (some portions): Apache 2.0, and 3-clause BSD
+
+Some portions of this module are derived from code in the Chromium project,
+copyright (c) Google inc and (c) The Chromium Authors and licensed under the
+Apache 2.0 License or the under the 3-clause BSD license:
+
+  Copyright (c) 2013 The Chromium Authors. All rights reserved.
+
+  Redistribution and use in source and binary forms, with or without
+  modification, are permitted provided that the following conditions are
+  met:
+
+     * Redistributions of source code must retain the above copyright
+  notice, this list of conditions and the following disclaimer.
+     * Redistributions in binary form must reproduce the above
+  copyright notice, this list of conditions and the following disclaimer
+  in the documentation and/or other materials provided with the
+  distribution.
+     * Neither the name of Google Inc. nor the names of its
+  contributors may be used to endorse or promote products derived from
+  this software without specific prior written permission.
+
+  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+  "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+  LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+  A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+  OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+  SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+  LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+  DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+  THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+  (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+  OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

http://git-wip-us.apache.org/repos/asf/arrow/blob/ec6e183e/cpp/src/arrow/builder-benchmark.cc
----------------------------------------------------------------------
diff --git a/cpp/src/arrow/builder-benchmark.cc b/cpp/src/arrow/builder-benchmark.cc
index 62f2fd6..3c49c63 100644
--- a/cpp/src/arrow/builder-benchmark.cc
+++ b/cpp/src/arrow/builder-benchmark.cc
@@ -25,6 +25,12 @@ namespace arrow {
 
 constexpr int64_t kFinalSize = 256;
 
+#define ABORT_NOT_OK(s)                                 \
+  do {                                                  \
+    ::arrow::Status _s = (s);                           \
+    if (ARROW_PREDICT_FALSE(!_s.ok())) { exit(-1); }    \
+  } while (0);
+
 static void BM_BuildPrimitiveArrayNoNulls(
     benchmark::State& state) {  // NOLINT non-const reference
   // 2 MiB block
@@ -33,17 +39,15 @@ static void BM_BuildPrimitiveArrayNoNulls(
     Int64Builder builder(default_memory_pool());
     for (int i = 0; i < kFinalSize; i++) {
       // Build up an array of 512 MiB in size
-      builder.Append(data.data(), data.size(), nullptr);
+      ABORT_NOT_OK(builder.Append(data.data(), data.size(), nullptr));
     }
     std::shared_ptr<Array> out;
-    builder.Finish(&out);
+    ABORT_NOT_OK(builder.Finish(&out));
   }
   state.SetBytesProcessed(
       state.iterations() * data.size() * sizeof(int64_t) * kFinalSize);
 }
 
-BENCHMARK(BM_BuildPrimitiveArrayNoNulls)->Repetitions(3)->Unit(benchmark::kMillisecond);
-
 static void BM_BuildVectorNoNulls(
     benchmark::State& state) {  // NOLINT non-const reference
   // 2 MiB block
@@ -59,8 +63,6 @@ static void BM_BuildVectorNoNulls(
       state.iterations() * data.size() * sizeof(int64_t) * kFinalSize);
 }
 
-BENCHMARK(BM_BuildVectorNoNulls)->Repetitions(3)->Unit(benchmark::kMillisecond);
-
 static void BM_BuildAdaptiveIntNoNulls(
     benchmark::State& state) {  // NOLINT non-const reference
   int64_t size = static_cast<int64_t>(std::numeric_limits<int16_t>::max()) *
256;
@@ -73,16 +75,14 @@ static void BM_BuildAdaptiveIntNoNulls(
     AdaptiveIntBuilder builder(default_memory_pool());
     for (int64_t i = 0; i < size; i += chunk_size) {
       // Build up an array of 512 MiB in size
-      builder.Append(data.data() + i, chunk_size, nullptr);
+      ABORT_NOT_OK(builder.Append(data.data() + i, chunk_size, nullptr));
     }
     std::shared_ptr<Array> out;
-    builder.Finish(&out);
+    ABORT_NOT_OK(builder.Finish(&out));
   }
   state.SetBytesProcessed(state.iterations() * data.size() * sizeof(int64_t));
 }
 
-BENCHMARK(BM_BuildAdaptiveIntNoNulls)->Repetitions(3)->Unit(benchmark::kMillisecond);
-
 static void BM_BuildAdaptiveIntNoNullsScalarAppend(
     benchmark::State& state) {  // NOLINT non-const reference
   int64_t size = static_cast<int64_t>(std::numeric_limits<int16_t>::max()) *
256;
@@ -93,18 +93,14 @@ static void BM_BuildAdaptiveIntNoNullsScalarAppend(
   while (state.KeepRunning()) {
     AdaptiveIntBuilder builder(default_memory_pool());
     for (int64_t i = 0; i < size; i++) {
-      builder.Append(data[i]);
+      ABORT_NOT_OK(builder.Append(data[i]));
     }
     std::shared_ptr<Array> out;
-    builder.Finish(&out);
+    ABORT_NOT_OK(builder.Finish(&out));
   }
   state.SetBytesProcessed(state.iterations() * data.size() * sizeof(int64_t));
 }
 
-BENCHMARK(BM_BuildAdaptiveIntNoNullsScalarAppend)
-    ->Repetitions(3)
-    ->Unit(benchmark::kMillisecond);
-
 static void BM_BuildAdaptiveUIntNoNulls(
     benchmark::State& state) {  // NOLINT non-const reference
   int64_t size = static_cast<int64_t>(std::numeric_limits<uint16_t>::max()) *
256;
@@ -117,14 +113,20 @@ static void BM_BuildAdaptiveUIntNoNulls(
     AdaptiveUIntBuilder builder(default_memory_pool());
     for (int64_t i = 0; i < size; i += chunk_size) {
       // Build up an array of 512 MiB in size
-      builder.Append(data.data() + i, chunk_size, nullptr);
+      ABORT_NOT_OK(builder.Append(data.data() + i, chunk_size, nullptr));
     }
     std::shared_ptr<Array> out;
-    builder.Finish(&out);
+    ABORT_NOT_OK(builder.Finish(&out));
   }
   state.SetBytesProcessed(state.iterations() * data.size() * sizeof(int64_t));
 }
 
-BENCHMARK(BM_BuildAdaptiveUIntNoNulls)->Repetitions(3)->Unit(benchmark::kMillisecond);
+BENCHMARK(BM_BuildPrimitiveArrayNoNulls)->Repetitions(3)->Unit(benchmark::kMicrosecond);
+BENCHMARK(BM_BuildVectorNoNulls)->Repetitions(3)->Unit(benchmark::kMicrosecond);
+BENCHMARK(BM_BuildAdaptiveIntNoNulls)->Repetitions(3)->Unit(benchmark::kMicrosecond);
+BENCHMARK(BM_BuildAdaptiveIntNoNullsScalarAppend)
+    ->Repetitions(3)
+    ->Unit(benchmark::kMicrosecond);
+BENCHMARK(BM_BuildAdaptiveUIntNoNulls)->Repetitions(3)->Unit(benchmark::kMicrosecond);
 
 }  // namespace arrow

http://git-wip-us.apache.org/repos/asf/arrow/blob/ec6e183e/cpp/src/arrow/status.h
----------------------------------------------------------------------
diff --git a/cpp/src/arrow/status.h b/cpp/src/arrow/status.h
index 7e7f67c..9a75a58 100644
--- a/cpp/src/arrow/status.h
+++ b/cpp/src/arrow/status.h
@@ -10,7 +10,7 @@
 // non-const method, all threads accessing the same Status must use
 // external synchronization.
 
-// Adapted from Kudu github.com/cloudera/kudu
+// Adapted from Kudu github.com/apache/kudu
 
 #ifndef ARROW_STATUS_H_
 #define ARROW_STATUS_H_
@@ -19,30 +19,14 @@
 #include <cstring>
 #include <string>
 
+#include "arrow/util/macros.h"
 #include "arrow/util/visibility.h"
 
 // Return the given status if it is not OK.
-#define ARROW_RETURN_NOT_OK(s)   \
-  do {                           \
-    ::arrow::Status _s = (s);    \
-    if (!_s.ok()) { return _s; } \
-  } while (0);
-
-// Return the given status if it is not OK, but first clone it and
-// prepend the given message.
-#define ARROW_RETURN_NOT_OK_PREPEND(s, msg)                               \
-  do {                                                                    \
-    ::arrow::Status _s = (s);                                             \
-    if (::gutil::PREDICT_FALSE(!_s.ok())) return _s.CloneAndPrepend(msg); \
-  } while (0);
-
-// Return 'to_return' if 'to_call' returns a bad status.
-// The substitution for 'to_return' may reference the variable
-// 's' for the bad status.
-#define ARROW_RETURN_NOT_OK_RET(to_call, to_return)          \
-  do {                                                       \
-    ::arrow::Status s = (to_call);                           \
-    if (::gutil::PREDICT_FALSE(!s.ok())) return (to_return); \
+#define ARROW_RETURN_NOT_OK(s)                          \
+  do {                                                  \
+    ::arrow::Status _s = (s);                           \
+    if (ARROW_PREDICT_FALSE(!_s.ok())) { return _s; }   \
   } while (0);
 
 // If 'to_call' returns a bad status, CHECK immediately with a logged message
@@ -59,10 +43,10 @@
 
 namespace arrow {
 
-#define RETURN_NOT_OK(s)         \
-  do {                           \
-    Status _s = (s);             \
-    if (!_s.ok()) { return _s; } \
+#define RETURN_NOT_OK(s)                                \
+  do {                                                  \
+    Status _s = (s);                                    \
+    if (ARROW_PREDICT_FALSE(!_s.ok())) { return _s; }   \
   } while (0);
 
 #define RETURN_NOT_OK_ELSE(s, else_) \

http://git-wip-us.apache.org/repos/asf/arrow/blob/ec6e183e/cpp/src/arrow/util/compression.cc
----------------------------------------------------------------------
diff --git a/cpp/src/arrow/util/compression.cc b/cpp/src/arrow/util/compression.cc
index f82ae5c..070f857 100644
--- a/cpp/src/arrow/util/compression.cc
+++ b/cpp/src/arrow/util/compression.cc
@@ -17,6 +17,11 @@
 
 #include "arrow/util/compression.h"
 
+// Work around warning caused by Snappy include
+#ifdef DISALLOW_COPY_AND_ASSIGN
+#undef DISALLOW_COPY_AND_ASSIGN
+#endif
+
 #include <cstdint>
 #include <memory>
 #include <sstream>

http://git-wip-us.apache.org/repos/asf/arrow/blob/ec6e183e/cpp/src/arrow/util/macros.h
----------------------------------------------------------------------
diff --git a/cpp/src/arrow/util/macros.h b/cpp/src/arrow/util/macros.h
index c4a62a4..affe8bc 100644
--- a/cpp/src/arrow/util/macros.h
+++ b/cpp/src/arrow/util/macros.h
@@ -27,4 +27,18 @@
 
 #define UNUSED(x) (void)x
 
+//
+// GCC can be told that a certain branch is not likely to be taken (for
+// instance, a CHECK failure), and use that information in static analysis.
+// Giving it this information can help it optimize for the common case in
+// the absence of better information (ie. -fprofile-arcs).
+//
+#if defined(__GNUC__)
+#define ARROW_PREDICT_FALSE(x) (__builtin_expect(x, 0))
+#define ARROW_PREDICT_TRUE(x) (__builtin_expect(!!(x), 1))
+#else
+#define ARROW_PREDICT_FALSE(x) x
+#define ARROW_PREDICT_TRUE(x) x
+#endif
+
 #endif  // ARROW_UTIL_MACROS_H


Mime
View raw message