Compare length + prefix together in ArrowBytesViewMap find#21345
Compare length + prefix together in ArrowBytesViewMap find#21345Dandandan wants to merge 1 commit intoapache:mainfrom
Conversation
For strings > 12 bytes, compare the first 8 bytes of the StringView (length + 4-byte prefix) as a single u64 instead of only comparing the 4-byte prefix. This rejects length mismatches earlier and is simpler. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
run benchmarks |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing bytes-view-map-length-prefix-compare (7522675) to 1e93a67 (merge-base) diff using: clickbench_partitioned File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing bytes-view-map-length-prefix-compare (7522675) to 1e93a67 (merge-base) diff using: tpcds File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing bytes-view-map-length-prefix-compare (7522675) to 1e93a67 (merge-base) diff using: tpch File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagetpch — base (merge-base)
tpch — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagetpcds — base (merge-base)
tpcds — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usageclickbench_partitioned — base (merge-base)
clickbench_partitioned — branch
File an issue against this benchmark runner |
Which issue does this PR close?
N/A - performance optimization
Rationale for this change
When probing
ArrowBytesViewMapfor strings > 12 bytes, the existing code only compared the 4-byte prefix before falling through to a full byte comparison. It did not compare the string length first, so two strings with different lengths but the same hash and prefix would unnecessarily perform a full memcmp.What changes are included in this PR?
Compare the first 8 bytes of the StringView (length + 4-byte prefix) as a single
u64instead of extracting and comparing only the 4-byte prefix. This:u64comparison vs oneu32extraction + comparison)Are these changes tested?
Existing tests cover this code path.
Are there any user-facing changes?
No, this is a performance optimization only.
🤖 Generated with Claude Code