Skip to content

Eliminate unnecessary refcount traffic from shared_ptr parameters passed by value that don't need to be #51270

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
hsutter wants to merge 3 commits into from
Closed

Eliminate unnecessary refcount traffic from shared_ptr parameters passed by value that don't need to be #51270

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
935de16
Claude's initial application of rule SP2 to `cpp/src/arrow/*`
hsutter Sep 8, 2026
52e6779
Fix compilation errors
hsutter Sep 8, 2026
f7556fe
Do the five 5-function cleanup items Claude recommended
hsutter Sep 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cpp/src/arrow/acero/asof_join_benchmark.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ static void TableJoinOverhead(benchmark::State& state,
TableGenerationProperties right_table_properties,
int batch_size, int num_right_tables,
std::string factory_name,
std::shared_ptr<ExecNodeOptions> options) {
const std::shared_ptr<ExecNodeOptions>& options) {
left_table_properties.column_prefix = "lt";
left_table_properties.seed = 0;
ASSERT_OK_AND_ASSIGN(TableStats left_table_stats, MakeTable(left_table_properties));
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/acero/asof_join_node.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ struct MemoStore {
struct Entry {
Entry() = default;

Entry(OnType time, std::shared_ptr<arrow::RecordBatch> batch, row_index_t row)
Entry(OnType time, const std::shared_ptr<arrow::RecordBatch>& batch, row_index_t row)
: time(time), batch(batch), row(row) {}

void swap(Entry& other) {
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/acero/asof_join_node_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1396,7 +1396,7 @@ struct BackpressureCountingNode : public MapNode {
}

BackpressureCountingNode(ExecPlan* plan, std::vector<ExecNode*> inputs,
std::shared_ptr<Schema> output_schema,
const std::shared_ptr<Schema>& output_schema,
const BackpressureCountingNodeOptions& options)
: MapNode(plan, inputs, output_schema), counters(options.counters) {}

Expand Down
4 changes: 2 additions & 2 deletions cpp/src/arrow/acero/exec_plan.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1150,10 +1150,10 @@ ExecFactoryRegistry* default_exec_factory_registry() {
}

Result<std::function<Future<std::optional<ExecBatch>>()>> MakeReaderGenerator(
std::shared_ptr<RecordBatchReader> reader, ::arrow::internal::Executor* io_executor,
const std::shared_ptr<RecordBatchReader>& reader, ::arrow::internal::Executor* io_executor,
int max_q, int q_restart) {
auto batch_it = MakeMapIterator(
[](std::shared_ptr<RecordBatch> batch) {
[](const std::shared_ptr<RecordBatch>& batch) {
return std::make_optional(ExecBatch(*batch));
},
MakeIteratorFromReader(reader));
Comment on lines +1156 to 1159

@pitrou pitrou Sep 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is right. MakeIteratorFromReader should instead take its argument by value instead of const-ref.

@hsutter hsutter Sep 11, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pitrou, thanks again.

Can you help me to understand this feedback please...

  1. I'm not sure how MakeIteratorFromReader is related. That's outside the lambda whose parameter is being changed here?

  2. The lambda only dereferences batch. Doesn't currently taking batch by value add a (needless?) inc/dec on the refcount? Isn't changing it to pass by const& a strict improvement?

Again, my apologies if I'm missing something here or asking a silly question! I don't know this code base well and I appreciate your feedback.

Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/acero/exec_plan.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,7 @@ constexpr int kDefaultBackgroundQRestart = 16;
/// Useful as a source node for an Exec plan
ARROW_ACERO_EXPORT
Result<std::function<Future<std::optional<ExecBatch>>()>> MakeReaderGenerator(
std::shared_ptr<RecordBatchReader> reader, arrow::internal::Executor* io_executor,
const std::shared_ptr<RecordBatchReader>& reader, arrow::internal::Executor* io_executor,
int max_q = kDefaultBackgroundMaxQ, int q_restart = kDefaultBackgroundQRestart);

} // namespace acero
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/acero/hash_aggregate_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ Result<Datum> NaiveGroupBy(std::vector<Datum> arguments, std::vector<Datum> keys
}

Result<Datum> MakeGroupByOutput(const std::vector<ExecBatch>& output_batches,
const std::shared_ptr<Schema> output_schema,
const std::shared_ptr<Schema>& output_schema,
size_t num_aggregates, size_t num_keys, bool naive) {
ArrayVector out_arrays(num_aggregates + num_keys);
for (size_t i = 0; i < out_arrays.size(); ++i) {
Expand Down
8 changes: 4 additions & 4 deletions cpp/src/arrow/acero/source_node.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ struct SourceNode : ExecNode, public TracedNode {
};

struct TableSourceNode : public SourceNode {
TableSourceNode(ExecPlan* plan, std::shared_ptr<Table> table, int64_t batch_size)
TableSourceNode(ExecPlan* plan, const std::shared_ptr<Table>& table, int64_t batch_size)
: SourceNode(plan, table->schema(), TableGenerator(*table, batch_size),
Ordering::Implicit()) {}

Expand All @@ -319,7 +319,7 @@ struct TableSourceNode : public SourceNode {

const char* kind_name() const override { return "TableSourceNode"; }

static arrow::Status ValidateTableSourceNodeInput(const std::shared_ptr<Table> table,
static arrow::Status ValidateTableSourceNodeInput(const std::shared_ptr<Table>& table,
const int64_t batch_size) {
if (table == nullptr) {
return Status::Invalid("TableSourceNode requires table which is not null");
Expand Down Expand Up @@ -370,7 +370,7 @@ struct TableSourceNode : public SourceNode {

template <typename This, typename Options>
struct SchemaSourceNode : public SourceNode {
SchemaSourceNode(ExecPlan* plan, std::shared_ptr<Schema> schema,
SchemaSourceNode(ExecPlan* plan, const std::shared_ptr<Schema>& schema,
arrow::AsyncGenerator<std::optional<ExecBatch>> generator)
: SourceNode(plan, schema, generator, Ordering::Implicit()) {}

Expand Down Expand Up @@ -406,7 +406,7 @@ struct SchemaSourceNode : public SourceNode {
};

struct RecordBatchReaderSourceNode : public SourceNode {
RecordBatchReaderSourceNode(ExecPlan* plan, std::shared_ptr<Schema> schema,
RecordBatchReaderSourceNode(ExecPlan* plan, const std::shared_ptr<Schema>& schema,
arrow::AsyncGenerator<std::optional<ExecBatch>> generator)
: SourceNode(plan, schema, generator, Ordering::Implicit()) {}

Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/acero/source_node_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ struct PauseThenStopNodeOptions : public ExecNodeOptions {
template <typename ThisNode>
struct PauseThenStopNode : public MapNode {
PauseThenStopNode(ExecPlan* plan, std::vector<ExecNode*> inputs,
std::shared_ptr<Schema> output_schema,
const std::shared_ptr<Schema>& output_schema,
const PauseThenStopNodeOptions& options)
: MapNode(plan, inputs, output_schema), num_pass(options.num_pass) {}

Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/acero/test_nodes.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ struct GatedNode : public ExecNode, public TracedNode {
}

GatedNode(ExecPlan* plan, std::vector<ExecNode*> inputs,
std::shared_ptr<Schema> output_schema, const GatedNodeOptions& options)
const std::shared_ptr<Schema>& output_schema, const GatedNodeOptions& options)

@pitrou pitrou Sep 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems wrong too: the ExecNode constructor takes output_schema by value. So the change should instead be to add a std::move.

@hsutter hsutter Sep 12, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again.

For reading convenience here's the whole function as it is today:

  GatedNode(ExecPlan* plan, std::vector<ExecNode*> inputs,
            std::shared_ptr<Schema> output_schema, const GatedNodeOptions& options)
      : ExecNode(plan, inputs, {"input"}, output_schema),
        TracedNode(this),
        gate_(options.gate) {}

First, I think we agree there's a performance bug? We agree this function should be changed and that there is a needless copy, right? This is a good thing!

Alternative 1 (this PR): The PR's proposed change to pass output_schema by const& will completely eliminate the needless copy:

  • Performance: It changes the needless copy to "nothing."
  • Readability: Adding const& declares in intent up front (on the declaration) and avoids disturbing the function body (no need to remember to be careful how to use the parameter). Personally I prefer declaring intent as simpler code to read and maintain.

Alternative 2 (add std::move() in the body): If instead we kept pass by value and added a std::move, that would also eliminate the needless copy too, but:

  • Performance: It would change a copy to a "move." That's still much cheaper than a copy for shared_ptr, but FWIW a move is still more expensive than "nothing."
  • Readability: Adding std::move() at each point of use requires remembering to do that the body (and it seems like we agree it's a problem that the current code doesn't do it, so maybe that's a proof point that it's easy to forget to do it?), and that the reader and maintainer remember that too which is the greater cost over time. IME that's a greater cost than declaring intent on the declaration?

Isn't this PR's suggestion worth considering, to change a copy to nothing at all and with arguably simpler code?

Again, sorry if I'm missing something! (In particular, I have no idea whether these signatures I'm proposing to change might be exported/API functions, e.g., for use in cross-language APIs, that must be pass by value and can't tolerate pass by const&. Is that an issue, and if so how can I tell which types/functions can't tolerate such a parameter passing change so I can exclude them?)

Thank you for your feedback.

: ExecNode(plan, inputs, {"input"}, output_schema),
TracedNode(this),
gate_(options.gate) {}
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/acero/unmaterialized_table_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ class UnmaterializedSliceBuilder {
UnmaterializedCompositeTable<MAX_COMPOSITE_TABLES>* table_)
: table(table_) {}

void AddEntry(std::shared_ptr<RecordBatch> rb, uint64_t start, uint64_t end) {
void AddEntry(const std::shared_ptr<RecordBatch>& rb, uint64_t start, uint64_t end) {
if (rb) {
table->AddRecordBatchRef(rb);
}
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/adapters/orc/adapter.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ constexpr int64_t kReadRowsBatch = 1000;
class OrcStripeReader : public RecordBatchReader {
public:
OrcStripeReader(std::unique_ptr<liborc::RowReader> row_reader,
std::shared_ptr<Schema> schema, int64_t batch_size, MemoryPool* pool)
const std::shared_ptr<Schema>& schema, int64_t batch_size, MemoryPool* pool)
: row_reader_(std::move(row_reader)),
schema_(schema),
Comment on lines +138 to 140

@pitrou pitrou Sep 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here: should move the schema constructor argument into the schema_ attribute instead.

@hsutter hsutter Sep 12, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would repeat the same considerations as in my reply about GatedNode... I try to make the case that (minor) adding a std::move is slightly less performance and (major) it's less simple/maintainable. Does that position seem reasonable?

pool_(pool),
Expand Down
10 changes: 5 additions & 5 deletions cpp/src/arrow/adapters/orc/adapter_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ std::shared_ptr<Buffer> GenerateFixedDifferenceBuffer(int32_t fixed_length,
}

std::shared_ptr<Array> CastFixedSizeBinaryArrayToBinaryArray(
std::shared_ptr<Array> array) {
const std::shared_ptr<Array>& array) {
auto fixed_size_binary_array = checked_pointer_cast<FixedSizeBinaryArray>(array);
std::shared_ptr<Buffer> value_offsets = GenerateFixedDifferenceBuffer(
fixed_size_binary_array->byte_width(), array->length() + 1);
Expand All @@ -114,7 +114,7 @@ std::shared_ptr<Array> CastFixedSizeBinaryArrayToBinaryArray(

template <typename TargetArrayType>
std::shared_ptr<Array> CastInt64ArrayToTemporalArray(
const std::shared_ptr<DataType>& type, std::shared_ptr<Array> array) {
const std::shared_ptr<DataType>& type, const std::shared_ptr<Array>& array) {
std::shared_ptr<ArrayData> new_array_data =
ArrayData::Make(type, array->length(), array->data()->buffers);
return std::make_shared<TargetArrayType>(new_array_data);
Expand All @@ -128,7 +128,7 @@ Result<std::shared_ptr<Array>> GenerateRandomDate64Array(int64_t size,
}

Result<std::shared_ptr<Array>> GenerateRandomTimestampArray(
int64_t size, std::shared_ptr<TimestampType> type, double null_probability) {
int64_t size, const std::shared_ptr<TimestampType>& type, double null_probability) {
random::RandomArrayGenerator rand(kRandomSeed);
switch (type->unit()) {
case TimeUnit::type::SECOND: {
Expand Down Expand Up @@ -1049,7 +1049,7 @@ namespace {
// read them back and compare equality in the unit test). Because the orc reader
// fills unselected values to nulls when reading from the file. So flattening
// the SparseUnionArray before writing makes it easy for the array equality check.
std::shared_ptr<Array> FlattenSparseUnionArray(std::shared_ptr<Array> array) {
std::shared_ptr<Array> FlattenSparseUnionArray(const std::shared_ptr<Array>& array) {
auto union_array = checked_pointer_cast<SparseUnionArray>(array);
ArrayVector children;
for (int i = 0; i < array->num_fields(); ++i) {
Expand All @@ -1061,7 +1061,7 @@ std::shared_ptr<Array> FlattenSparseUnionArray(std::shared_ptr<Array> array) {
union_array->type_codes(), array->offset());
}

void TestUnionConversion(std::shared_ptr<Array> array) {
void TestUnionConversion(const std::shared_ptr<Array>& array) {
auto length = array->length();
auto orc_type = liborc::Type::buildTypeFromString("uniontype<string,int>");

Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/adapters/tensorflow/convert.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ Status GetArrowType(::tensorflow::DataType dtype, std::shared_ptr<DataType>* out
return Status::OK();
}

Status GetTensorFlowType(std::shared_ptr<DataType> dtype, ::tensorflow::DataType* out) {
Status GetTensorFlowType(const std::shared_ptr<DataType>& dtype, ::tensorflow::DataType* out) {
switch (dtype->id()) {
case Type::BOOL:
*out = ::tensorflow::DT_BOOL;
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/array/array_dict.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ struct RecursiveUnifier {
} // namespace

Result<std::unique_ptr<DictionaryUnifier>> DictionaryUnifier::Make(
std::shared_ptr<DataType> value_type, MemoryPool* pool) {
const std::shared_ptr<DataType>& value_type, MemoryPool* pool) {
MakeUnifier maker(pool, value_type);
RETURN_NOT_OK(VisitTypeInline(*value_type, &maker));
return std::move(maker.result);
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/array/array_dict.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ class ARROW_EXPORT DictionaryUnifier {
/// \param[in] value_type the data type of the dictionaries
/// \param[in] pool MemoryPool to use for memory allocations
static Result<std::unique_ptr<DictionaryUnifier>> Make(
std::shared_ptr<DataType> value_type, MemoryPool* pool = default_memory_pool());
const std::shared_ptr<DataType>& value_type, MemoryPool* pool = default_memory_pool());

/// \brief Unify dictionaries across array chunks
///
Expand Down
8 changes: 4 additions & 4 deletions cpp/src/arrow/array/array_dict_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -873,7 +873,7 @@ TEST(TestFixedSizeBinaryDictionaryBuilder, AppendArrayInvalidType) {
#endif

template <typename DecimalValue>
void TestDecimalDictionaryBuilderBasic(std::shared_ptr<DataType> decimal_type) {
void TestDecimalDictionaryBuilderBasic(const std::shared_ptr<DataType>& decimal_type) {
// Build the dictionary Array
DictionaryBuilder<FixedSizeBinaryType> builder(decimal_type);

Expand Down Expand Up @@ -903,7 +903,7 @@ TEST(TestDecimal256DictionaryBuilder, Basic) {
}

void TestDecimalDictionaryBuilderDoubleTableSize(
std::shared_ptr<DataType> decimal_type, FixedSizeBinaryBuilder& decimal_builder) {
const std::shared_ptr<DataType>& decimal_type, FixedSizeBinaryBuilder& decimal_builder) {
// Build the dictionary Array
DictionaryBuilder<FixedSizeBinaryType> dict_builder(decimal_type);

Expand Down Expand Up @@ -1488,8 +1488,8 @@ TEST(TestDictionary, ListOfDictionary) {
}

TEST(TestDictionary, CanCompareIndices) {
auto make_dict = [](std::shared_ptr<DataType> index_type,
std::shared_ptr<DataType> value_type, std::string dictionary_json) {
auto make_dict = [](const std::shared_ptr<DataType>& index_type,
const std::shared_ptr<DataType>& value_type, std::string dictionary_json) {
std::shared_ptr<Array> out;
ARROW_EXPECT_OK(
DictionaryArray::FromArrays(dictionary(index_type, value_type),
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/array/array_list_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ class TestListArray : public ::testing::Test {
private:
Result<std::shared_ptr<Array>> FromArrays(const Array& offsets, const Array& sizes,
const Array& values,
std::shared_ptr<Buffer> null_bitmap = NULLPTR,
const std::shared_ptr<Buffer>& null_bitmap = NULLPTR,
int64_t null_count = kUnknownNullCount) {
if constexpr (kTypeClassIsListView) {
return ArrayType::FromArrays(offsets, sizes, values, pool_, null_bitmap,
Expand Down
10 changes: 5 additions & 5 deletions cpp/src/arrow/array/array_nested.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ Result<BufferVector> CleanListOffsets(const std::shared_ptr<Buffer>& validity_bu

template <typename TYPE>
Result<std::shared_ptr<typename TypeTraits<TYPE>::ArrayType>> ListArrayFromArrays(
std::shared_ptr<DataType> type, const Array& offsets, const Array& values,
const std::shared_ptr<DataType>& type, const Array& offsets, const Array& values,
MemoryPool* pool, std::shared_ptr<Buffer> null_bitmap = NULLPTR,
int64_t null_count = kUnknownNullCount) {
using offset_type = typename TYPE::offset_type;
Expand Down Expand Up @@ -145,7 +145,7 @@ Result<std::shared_ptr<typename TypeTraits<TYPE>::ArrayType>> ListArrayFromArray

template <typename TYPE>
Result<std::shared_ptr<typename TypeTraits<TYPE>::ArrayType>> ListViewArrayFromArrays(
std::shared_ptr<DataType> type, const Array& offsets, const Array& sizes,
const std::shared_ptr<DataType>& type, const Array& offsets, const Array& sizes,
const Array& values, MemoryPool* pool, std::shared_ptr<Buffer> null_bitmap = NULLPTR,
int64_t null_count = kUnknownNullCount) {
using offset_type = typename TYPE::offset_type;
Expand Down Expand Up @@ -524,7 +524,7 @@ ListArray::ListArray(std::shared_ptr<ArrayData> data) {
}

ListArray::ListArray(std::shared_ptr<DataType> type, int64_t length,
std::shared_ptr<Buffer> value_offsets, std::shared_ptr<Array> values,
std::shared_ptr<Buffer> value_offsets, const std::shared_ptr<Array>& values,
std::shared_ptr<Buffer> null_bitmap, int64_t null_count,
int64_t offset) {
ARROW_CHECK_EQ(type->id(), Type::LIST);
Expand Down Expand Up @@ -644,7 +644,7 @@ ListViewArray::ListViewArray(std::shared_ptr<ArrayData> data) {
ListViewArray::ListViewArray(std::shared_ptr<DataType> type, int64_t length,
std::shared_ptr<Buffer> value_offsets,
std::shared_ptr<Buffer> value_sizes,
std::shared_ptr<Array> values,
const std::shared_ptr<Array>& values,
std::shared_ptr<Buffer> null_bitmap, int64_t null_count,
int64_t offset) {
ListViewArray::SetData(ArrayData::Make(
Expand Down Expand Up @@ -719,7 +719,7 @@ LargeListViewArray::LargeListViewArray(std::shared_ptr<ArrayData> data) {
LargeListViewArray::LargeListViewArray(std::shared_ptr<DataType> type, int64_t length,
std::shared_ptr<Buffer> value_offsets,
std::shared_ptr<Buffer> value_sizes,
std::shared_ptr<Array> values,
const std::shared_ptr<Array>& values,
std::shared_ptr<Buffer> null_bitmap,
int64_t null_count, int64_t offset) {
LargeListViewArray::SetData(ArrayData::Make(
Expand Down
6 changes: 3 additions & 3 deletions cpp/src/arrow/array/array_nested.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ class ARROW_EXPORT ListArray : public BaseListArray<ListType> {
explicit ListArray(std::shared_ptr<ArrayData> data);

ListArray(std::shared_ptr<DataType> type, int64_t length,
std::shared_ptr<Buffer> value_offsets, std::shared_ptr<Array> values,
std::shared_ptr<Buffer> value_offsets, const std::shared_ptr<Array>& values,
std::shared_ptr<Buffer> null_bitmap = NULLPTR,
int64_t null_count = kUnknownNullCount, int64_t offset = 0);

Expand Down Expand Up @@ -319,7 +319,7 @@ class ARROW_EXPORT ListViewArray : public BaseListViewArray<ListViewType> {

ListViewArray(std::shared_ptr<DataType> type, int64_t length,
std::shared_ptr<Buffer> value_offsets,
std::shared_ptr<Buffer> value_sizes, std::shared_ptr<Array> values,
std::shared_ptr<Buffer> value_sizes, const std::shared_ptr<Array>& values,
std::shared_ptr<Buffer> null_bitmap = NULLPTR,
int64_t null_count = kUnknownNullCount, int64_t offset = 0);

Expand Down Expand Up @@ -407,7 +407,7 @@ class ARROW_EXPORT LargeListViewArray : public BaseListViewArray<LargeListViewTy

LargeListViewArray(std::shared_ptr<DataType> type, int64_t length,
std::shared_ptr<Buffer> value_offsets,
std::shared_ptr<Buffer> value_sizes, std::shared_ptr<Array> values,
std::shared_ptr<Buffer> value_sizes, const std::shared_ptr<Array>& values,
std::shared_ptr<Buffer> null_bitmap = NULLPTR,
int64_t null_count = kUnknownNullCount, int64_t offset = 0);

Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/array/array_run_end_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class TestRunEndEncodedArray
}

std::shared_ptr<RunEndEncodedArray> RunEndEncodedArrayFromJSON(
int64_t logical_length, std::shared_ptr<DataType> value_type,
int64_t logical_length, const std::shared_ptr<DataType>& value_type,
std::string_view run_ends_json, std::string_view values_json,
int64_t logical_offset = 0) {
auto run_ends = ArrayFromJSON(run_end_type, run_ends_json);
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/array/array_union_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ TEST(TestUnionArray, TestSliceEquals) {
std::shared_ptr<RecordBatch> batch;
ASSERT_OK(ipc::test::MakeUnion(&batch));

auto CheckUnion = [](std::shared_ptr<Array> array) {
auto CheckUnion = [](const std::shared_ptr<Array>& array) {
const int64_t size = array->length();
std::shared_ptr<Array> slice, slice2;
slice = array->Slice(2);
Expand Down
4 changes: 2 additions & 2 deletions cpp/src/arrow/array/builder_run_end.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ namespace internal {

RunCompressorBuilder::RunCompressorBuilder(MemoryPool* pool,
std::shared_ptr<ArrayBuilder> inner_builder,
std::shared_ptr<DataType> type)
const std::shared_ptr<DataType>& type)
: ArrayBuilder(pool), inner_builder_(std::move(inner_builder)) {}

RunCompressorBuilder::~RunCompressorBuilder() = default;
Expand Down Expand Up @@ -166,7 +166,7 @@ RunEndEncodedBuilder::ValueRunBuilder::ValueRunBuilder(

RunEndEncodedBuilder::RunEndEncodedBuilder(
MemoryPool* pool, const std::shared_ptr<ArrayBuilder>& run_end_builder,
const std::shared_ptr<ArrayBuilder>& value_builder, std::shared_ptr<DataType> type)
const std::shared_ptr<ArrayBuilder>& value_builder, const std::shared_ptr<DataType>& type)
: ArrayBuilder(pool), type_(internal::checked_pointer_cast<RunEndEncodedType>(type)) {
auto value_run_builder =
std::make_shared<ValueRunBuilder>(pool, value_builder, type_->value_type(), *this);
Expand Down
4 changes: 2 additions & 2 deletions cpp/src/arrow/array/builder_run_end.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ namespace internal {
class RunCompressorBuilder : public ArrayBuilder {
public:
RunCompressorBuilder(MemoryPool* pool, std::shared_ptr<ArrayBuilder> inner_builder,
std::shared_ptr<DataType> type);
const std::shared_ptr<DataType>& type);

~RunCompressorBuilder() override;

Expand Down Expand Up @@ -202,7 +202,7 @@ class ARROW_EXPORT RunEndEncodedBuilder : public ArrayBuilder {
RunEndEncodedBuilder(MemoryPool* pool,
const std::shared_ptr<ArrayBuilder>& run_end_builder,
const std::shared_ptr<ArrayBuilder>& value_builder,
std::shared_ptr<DataType> type);
const std::shared_ptr<DataType>& type);

/// \brief Allocate enough memory for a given number of array elements.
///
Expand Down
Loading
Loading