apacheGH-35611: [C++] Remove unnecessary safe operations for ListBuil…

…der and BinaryBuilder
js8544 · May 10, 2023 · c49516b · c49516b
1 parent 8be70c1
commit c49516b
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 14 deletions.
diff --git a/cpp/src/arrow/array/builder_binary.h b/cpp/src/arrow/array/builder_binary.h
@@ -67,7 +67,7 @@ class BaseBinaryBuilder
 
   Status Append(const uint8_t* value, offset_type length) {
     ARROW_RETURN_NOT_OK(Reserve(1));
-    ARROW_RETURN_NOT_OK(AppendNextOffset());
+    UnsafeAppendNextOffset();
     // Safety check for UBSAN.
     if (ARROW_PREDICT_TRUE(length > 0)) {
       ARROW_RETURN_NOT_OK(ValidateOverflow(length));
@@ -114,15 +114,15 @@ class BaseBinaryBuilder
   }
 
   Status AppendNull() final {
-    ARROW_RETURN_NOT_OK(AppendNextOffset());
     ARROW_RETURN_NOT_OK(Reserve(1));
+    UnsafeAppendNextOffset();
     UnsafeAppendToBitmap(false);
     return Status::OK();
   }
 
   Status AppendEmptyValue() final {
-    ARROW_RETURN_NOT_OK(AppendNextOffset());
     ARROW_RETURN_NOT_OK(Reserve(1));
+    UnsafeAppendNextOffset();
     UnsafeAppendToBitmap(true);
     return Status::OK();
   }
@@ -193,8 +193,7 @@ class BaseBinaryBuilder
         values.begin(), values.end(), 0ULL,
         [](uint64_t sum, const std::string& str) { return sum + str.size(); });
     ARROW_RETURN_NOT_OK(Reserve(values.size()));
-    ARROW_RETURN_NOT_OK(value_data_builder_.Reserve(total_length));
-    ARROW_RETURN_NOT_OK(offsets_builder_.Reserve(values.size()));
+    ARROW_RETURN_NOT_OK(ReserveData(total_length));
 
     if (valid_bytes != NULLPTR) {
       for (std::size_t i = 0; i < values.size(); ++i) {
@@ -288,13 +287,16 @@ class BaseBinaryBuilder
     auto bitmap = array.GetValues<uint8_t>(0, 0);
     auto offsets = array.GetValues<offset_type>(1);
     auto data = array.GetValues<uint8_t>(2, 0);
+    auto total_length = offsets[offset + length] - offsets[offset];
+    RETURN_NOT_OK(Reserve(length));
+    RETURN_NOT_OK(ReserveData(total_length));
     for (int64_t i = 0; i < length; i++) {
       if (!bitmap || bit_util::GetBit(bitmap, array.offset + offset + i)) {
         const offset_type start = offsets[offset + i];
         const offset_type end = offsets[offset + i + 1];
-        ARROW_RETURN_NOT_OK(Append(data + start, end - start));
+        UnsafeAppend(data + start, end - start);
       } else {
-        ARROW_RETURN_NOT_OK(AppendNull());
+        UnsafeAppendNull();
       }
     }
     return Status::OK();

diff --git a/cpp/src/arrow/array/builder_nested.h b/cpp/src/arrow/array/builder_nested.h
@@ -63,7 +63,7 @@ class BaseListBuilder : public ArrayBuilder {
       : BaseListBuilder(pool, value_builder, list(value_builder->type()), alignment) {}
 
   Status Resize(int64_t capacity) override {
-    if (capacity > maximum_elements()) {
+    if (ARROW_PREDICT_FALSE(capacity > maximum_elements())) {
       return Status::CapacityError("List array cannot reserve space for more than ",
                                    maximum_elements(), " got ", capacity);
     }
@@ -99,14 +99,14 @@ class BaseListBuilder : public ArrayBuilder {
   Status Append(bool is_valid = true) {
     ARROW_RETURN_NOT_OK(Reserve(1));
     UnsafeAppendToBitmap(is_valid);
-    return AppendNextOffset();
+    UnsafeAppendNextOffset();
+    return Status::OK();
   }
 
   Status AppendNull() final { return Append(false); }
 
   Status AppendNulls(int64_t length) final {
     ARROW_RETURN_NOT_OK(Reserve(length));
-    ARROW_RETURN_NOT_OK(ValidateOverflow(0));
     UnsafeAppendToBitmap(length, false);
     const int64_t num_values = value_builder_->length();
     for (int64_t i = 0; i < length; ++i) {
@@ -119,7 +119,6 @@ class BaseListBuilder : public ArrayBuilder {
 
   Status AppendEmptyValues(int64_t length) final {
     ARROW_RETURN_NOT_OK(Reserve(length));
-    ARROW_RETURN_NOT_OK(ValidateOverflow(0));
     UnsafeAppendToBitmap(length, true);
     const int64_t num_values = value_builder_->length();
     for (int64_t i = 0; i < length; ++i) {
@@ -133,17 +132,17 @@ class BaseListBuilder : public ArrayBuilder {
     const offset_type* offsets = array.GetValues<offset_type>(1);
     const bool all_valid = !array.MayHaveLogicalNulls();
     const uint8_t* validity = array.HasValidityBitmap() ? array.buffers[0].data : NULLPTR;
+    ARROW_RETURN_NOT_OK(Reserve(length));
     for (int64_t row = offset; row < offset + length; row++) {
       const bool is_valid =
           all_valid || (validity && bit_util::GetBit(validity, array.offset + row)) ||
           array.IsValid(row);
+      UnsafeAppendToBitmap(is_valid);
+      UnsafeAppendNextOffset();
       if (is_valid) {
-        ARROW_RETURN_NOT_OK(Append());
         int64_t slot_length = offsets[row + 1] - offsets[row];
         ARROW_RETURN_NOT_OK(value_builder_->AppendArraySlice(array.child_data[0],
                                                              offsets[row], slot_length));
-      } else {
-        ARROW_RETURN_NOT_OK(AppendNull());
       }
     }
     return Status::OK();
@@ -202,6 +201,11 @@ class BaseListBuilder : public ArrayBuilder {
     const int64_t num_values = value_builder_->length();
     return offsets_builder_.Append(static_cast<offset_type>(num_values));
   }
+
+  void UnsafeAppendNextOffset() {
+    const int64_t num_values = value_builder_->length();
+    offsets_builder_.UnsafeAppend(static_cast<offset_type>(num_values));
+  }
 };
 
 /// \class ListBuilder