Use strnlen to compute string length. Inline BinaryBuilder::AppendNextOffset

Change-Id: Ibfc09617b365c937e7af6a4943c274843f6e7a33

cpp/cmake_modules/SetupCxxFlags.cmake

@@ -25,6 +25,9 @@ CHECK_CXX_COMPILER_FLAG("-maltivec" CXX_SUPPORTS_ALTIVEC)
 # Arm64 compiler flags
 CHECK_CXX_COMPILER_FLAG("-march=armv8-a+crc" CXX_SUPPORTS_ARMCRC)
 
+# Support C11
+set(CMAKE_C_STANDARD 11)
+
 # This ensures that things like gnu++11 get passed correctly
 set(CMAKE_CXX_STANDARD 11)
 

cpp/src/arrow/array/builder_binary.cc

@@ -69,15 +69,11 @@ Status BinaryBuilder::ReserveData(int64_t elements) {
   return Status::OK();
 }
 
-Status BinaryBuilder::AppendNextOffset() {
-  const int64_t num_bytes = value_data_builder_.length();
-  if (ARROW_PREDICT_FALSE(num_bytes > kBinaryMemoryLimit)) {
-    std::stringstream ss;
-    ss << "BinaryArray cannot contain more than " << kBinaryMemoryLimit << " bytes, have "
-       << num_bytes;
-    return Status::CapacityError(ss.str());
-  }
-  return offsets_builder_.Append(static_cast<int32_t>(num_bytes));
+Status BinaryBuilder::AppendOverflow(int32_t num_bytes) {
+  std::stringstream ss;
+  ss << "BinaryArray cannot contain more than " << kBinaryMemoryLimit << " bytes, have "
+     << num_bytes;
+  return Status::CapacityError(ss.str());
 }
 
 Status BinaryBuilder::FinishInternal(std::shared_ptr<ArrayData>* out) {

cpp/src/arrow/array/builder_binary.h

@@ -19,6 +19,7 @@
 
 #include <limits>
 #include <memory>
+#include <sstream>
 #include <string>
 #include <vector>
 
@@ -120,7 +121,15 @@ class ARROW_EXPORT BinaryBuilder : public ArrayBuilder {
   TypedBufferBuilder<int32_t> offsets_builder_;
   TypedBufferBuilder<uint8_t> value_data_builder_;
 
-  Status AppendNextOffset();
+  Status AppendOverflow(int32_t num_bytes);
+
+  Status AppendNextOffset() {
+    const int64_t num_bytes = value_data_builder_.length();
+    if (ARROW_PREDICT_FALSE(num_bytes > kBinaryMemoryLimit)) {
+      return AppendOverflow(num_bytes);
+    }
+    return offsets_builder_.Append(static_cast<int32_t>(num_bytes));
+  }
 
   void UnsafeAppendNextOffset() {
     const int64_t num_bytes = value_data_builder_.length();

cpp/src/arrow/builder-benchmark.cc

@@ -163,10 +163,11 @@ static void BM_BuildBooleanArrayNoNulls(
 }
 
 static void BM_BuildBinaryArray(benchmark::State& state) {  // NOLINT non-const reference
-  const int64_t iterations = 1 << 20;
-
+  // About 160MB
+  const int64_t iterations = 1 << 24;
   std::string value = "1234567890";
-  while (state.KeepRunning()) {
+
+  for (auto _ : state) {
     BinaryBuilder builder;
     for (int64_t i = 0; i < iterations; i++) {
       ABORT_NOT_OK(builder.Append(value));
@@ -179,10 +180,11 @@ static void BM_BuildBinaryArray(benchmark::State& state) {  // NOLINT non-const
 
 static void BM_BuildChunkedBinaryArray(
     benchmark::State& state) {  // NOLINT non-const reference
-  const int64_t iterations = 1 << 20;
-
+  // About 160MB
+  const int64_t iterations = 1 << 24;
   std::string value = "1234567890";
-  while (state.KeepRunning()) {
+
+  for (auto _ : state) {
     // 1MB chunks
     const int32_t chunksize = 1 << 20;
     internal::ChunkedBinaryBuilder builder(chunksize);
@@ -390,10 +392,8 @@ BENCHMARK(BM_BuildAdaptiveUIntNoNullsScalarAppend)
     ->Repetitions(kRepetitions)
     ->Unit(benchmark::kMicrosecond);
 
-BENCHMARK(BM_BuildBinaryArray)->Repetitions(kRepetitions)->Unit(benchmark::kMicrosecond);
-BENCHMARK(BM_BuildChunkedBinaryArray)
-    ->Repetitions(kRepetitions)
-    ->Unit(benchmark::kMicrosecond);
+BENCHMARK(BM_BuildBinaryArray)->MinTime(1.0)->Unit(benchmark::kMicrosecond);
+BENCHMARK(BM_BuildChunkedBinaryArray)->MinTime(1.0)->Unit(benchmark::kMicrosecond);
 BENCHMARK(BM_BuildFixedSizeBinaryArray)
     ->Repetitions(kRepetitions)
     ->Unit(benchmark::kMicrosecond);

cpp/src/arrow/python/numpy_to_arrow.cc

@@ -25,6 +25,7 @@
 #include <algorithm>
 #include <cmath>
 #include <cstdint>
+#include <cstring>
 #include <limits>
 #include <memory>
 #include <sstream>
@@ -542,13 +543,9 @@ Status NumPyConverter::Visit(const BinaryType& type) {
   auto AppendNotNull = [&builder, this](const uint8_t* data) {
     // This is annoying. NumPy allows strings to have nul-terminators, so
     // we must check for them here
-    int item_length = 0;
-    for (; item_length < itemsize_; ++item_length) {
-      if (data[item_length] == 0) {
-        break;
-      }
-    }
-    return builder.Append(data, item_length);
+    const size_t item_size =
+        strnlen(reinterpret_cast<const char*>(data), static_cast<size_t>(itemsize_));
+    return builder.Append(data, static_cast<int32_t>(item_size));
   };
 
   if (mask_ != nullptr) {