You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "pitrou (via GitHub)" <gi...@apache.org> on 2026/05/11 13:47:53 UTC
[PR] GH-49959: [C++][Parquet] Avoid unbounded temp alloc in BYTE_STREAM_SPLIT decoder [arrow]
pitrou opened a new pull request, #49960:
URL: https://github.com/apache/arrow/pull/49960
### Rationale for this change
The BYTE_STREAM_SPLIT encoder and decoder allocate a `SmallVector` to hold the addresses of the different streams. While this is valid for small number of streams (which is most or all legitimate use cases), very large numbers of streams (e.g. FLBA(100000000)) can trigger a huge temporary memory allocation.
This issue was found by OSS-Fuzz: https://issues.oss-fuzz.com/issues/511575321
### What changes are included in this PR?
### Are these changes tested?
### Are there any user-facing changes?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-49959: [C++][Parquet] Avoid unbounded temp alloc in BYTE_STREAM_SPLIT decoder [arrow]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #49960:
URL: https://github.com/apache/arrow/pull/49960#discussion_r3224845392
##########
cpp/src/arrow/util/byte_stream_split_internal.h:
##########
@@ -421,14 +421,28 @@ void ByteStreamSplitEncodeScalar(const uint8_t* raw_values, int width,
DoSplitStreams(raw_values, kNumStreams, num_values, dest_streams.data());
}
+// If changing this value, please check that TestByteStreamSplitLargeWidth still
+// exercises the slow path.
+constexpr inline int kByteStreamSplitMaxTemporaryAlloc = 8192;
+
inline void ByteStreamSplitEncodeScalarDynamic(const uint8_t* raw_values, int width,
const int64_t num_values, uint8_t* out) {
- ::arrow::internal::SmallVector<uint8_t*, 16> dest_streams;
- dest_streams.resize(width);
- for (int stream = 0; stream < width; ++stream) {
- dest_streams[stream] = &out[stream * num_values];
+ if (ARROW_PREDICT_TRUE(width < kByteStreamSplitMaxTemporaryAlloc / 8)) {
+ ::arrow::internal::SmallVector<uint8_t*, 32> dest_streams;
Review Comment:
Why the small vector size is changed to 32 here but ByteStreamSplitDecodeScalarDynamic still uses 16?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-49959: [C++][Parquet] Avoid unbounded temp alloc in BYTE_STREAM_SPLIT decoder [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #49960:
URL: https://github.com/apache/arrow/pull/49960#issuecomment-4433145299
After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit a3f3d8bb0b5386d3c5390db0575398942d156a06.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/75628705036) has more details. It also includes information about 2 possible false positives for unstable benchmarks that are known to sometimes produce them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-49959: [C++][Parquet] Avoid unbounded temp alloc in BYTE_STREAM_SPLIT decoder [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #49960:
URL: https://github.com/apache/arrow/pull/49960#issuecomment-4421495383
Revision: 14e16dbe1cb26737b5ddec9845fffce3bbb8c6ba
Submitted crossbow builds: [ursacomputing/crossbow @ actions-51f44d0cee](https://github.com/ursacomputing/crossbow/branches/all?query=actions-51f44d0cee)
|Task|Status|
|----|------|
|example-cpp-minimal-build-static|[](https://github.com/ursacomputing/crossbow/actions/runs/25675403156/job/75371747247)|
|example-cpp-minimal-build-static-system-dependency|[](https://github.com/ursacomputing/crossbow/actions/runs/25675401891/job/75371743350)|
|example-cpp-tutorial|[](https://github.com/ursacomputing/crossbow/actions/runs/25675402006/job/75371743769)|
|test-build-cpp-fuzz|[](https://github.com/ursacomputing/crossbow/actions/runs/25675403765/job/75371750327)|
|test-conda-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/25675403304/job/75371747617)|
|test-conda-cpp-valgrind|[](https://github.com/ursacomputing/crossbow/actions/runs/25675400886/job/75371740481)|
|test-debian-13-cpp-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/25675401600/job/75371742709)|
|test-debian-13-cpp-i386|[](https://github.com/ursacomputing/crossbow/actions/runs/25675402898/job/75371747602)|
|test-debian-experimental-cpp-gcc-15|[](https://github.com/ursacomputing/crossbow/actions/runs/25675403871/job/75371751692)|
|test-fedora-42-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/25675403712/job/75371749200)|
|test-ubuntu-22.04-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/25675401306/job/75371741647)|
|test-ubuntu-22.04-cpp-bundled|[](https://github.com/ursacomputing/crossbow/actions/runs/25675402791/job/75371746224)|
|test-ubuntu-22.04-cpp-emscripten|[](https://github.com/ursacomputing/crossbow/actions/runs/25675401129/job/75371741053)|
|test-ubuntu-22.04-cpp-no-threading|[](https://github.com/ursacomputing/crossbow/actions/runs/25675404656/job/75371753752)|
|test-ubuntu-24.04-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/25675400647/job/75371739780)|
|test-ubuntu-24.04-cpp-bundled-offline|[](https://github.com/ursacomputing/crossbow/actions/runs/25675402982/job/75371747371)|
|test-ubuntu-24.04-cpp-gcc-13-bundled|[](https://github.com/ursacomputing/crossbow/actions/runs/25675400650/job/75371739446)|
|test-ubuntu-24.04-cpp-gcc-14|[](https://github.com/ursacomputing/crossbow/actions/runs/25675403973/job/75371750632)|
|test-ubuntu-24.04-cpp-minimal-with-formats|[](https://github.com/ursacomputing/crossbow/actions/runs/25675402499/job/75371746333)|
|test-ubuntu-24.04-cpp-thread-sanitizer|[](https://github.com/ursacomputing/crossbow/actions/runs/25675403995/job/75371750540)|
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-49959: [C++][Parquet] Avoid unbounded temp alloc in BYTE_STREAM_SPLIT decoder [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #49960:
URL: https://github.com/apache/arrow/pull/49960#issuecomment-4421474670
@github-actions crossbow submit -g cpp
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-49959: [C++][Parquet] Avoid unbounded temp alloc in BYTE_STREAM_SPLIT decoder [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou merged PR #49960:
URL: https://github.com/apache/arrow/pull/49960
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-49959: [C++][Parquet] Avoid unbounded temp alloc in BYTE_STREAM_SPLIT decoder [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #49960:
URL: https://github.com/apache/arrow/pull/49960#discussion_r3225039392
##########
cpp/src/arrow/util/byte_stream_split_internal.h:
##########
@@ -421,14 +421,28 @@ void ByteStreamSplitEncodeScalar(const uint8_t* raw_values, int width,
DoSplitStreams(raw_values, kNumStreams, num_values, dest_streams.data());
}
+// If changing this value, please check that TestByteStreamSplitLargeWidth still
+// exercises the slow path.
+constexpr inline int kByteStreamSplitMaxTemporaryAlloc = 8192;
+
inline void ByteStreamSplitEncodeScalarDynamic(const uint8_t* raw_values, int width,
const int64_t num_values, uint8_t* out) {
- ::arrow::internal::SmallVector<uint8_t*, 16> dest_streams;
- dest_streams.resize(width);
- for (int stream = 0; stream < width; ++stream) {
- dest_streams[stream] = &out[stream * num_values];
+ if (ARROW_PREDICT_TRUE(width < kByteStreamSplitMaxTemporaryAlloc / 8)) {
+ ::arrow::internal::SmallVector<uint8_t*, 32> dest_streams;
Review Comment:
Er... that's a good question. I probably thought it didn't hurt to bump it slightly, but then did not think about applying the same change to the decode routine. I'll make them consistent.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-49959: [C++][Parquet] Avoid unbounded temp alloc in BYTE_STREAM_SPLIT decoder [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #49960:
URL: https://github.com/apache/arrow/pull/49960#issuecomment-4429449381
Thanks for the review @wgtmac !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-49959: [C++][Parquet] Avoid unbounded temp alloc in BYTE_STREAM_SPLIT decoder [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #49960:
URL: https://github.com/apache/arrow/pull/49960#issuecomment-4450606547
After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit a3f3d8bb0b5386d3c5390db0575398942d156a06.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/75986208368) has more details.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org