Re: [PR] perf: bypass values.value(i) for inline strings in ArrowBytesViewMap [datafusion]
Dandandan merged PR #22172: URL: https://github.com/apache/datafusion/pull/22172 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] perf: bypass values.value(i) for inline strings in ArrowBytesViewMap [datafusion]
Dandandan commented on code in PR #22172:
URL: https://github.com/apache/datafusion/pull/22172#discussion_r3249321326
##
datafusion/physical-expr-common/src/binary_view_map.rs:
##
@@ -339,11 +339,31 @@ where
payload
} else {
// no existing value, make a new one
-let value: &[u8] = values.value(i).as_ref();
-let payload = make_payload_fn(Some(value));
+let (new_view, payload) = if len <= 12 {
+// Inline path: bytes are already packed in view_u128.
+// The inline ByteView format is [len:u32 LE][data:12
bytes zero-padded],
+// so extracting bytes from the u128 avoids a round-trip
through
+// values.value(i) (which reads the views buffer and
returns the same slice).
+let view_bytes = view_u128.to_le_bytes();
+let value = &view_bytes[4..4 + len as usize];
+let payload = make_payload_fn(Some(value));
+// For inline strings, the stored view is identical to the
input view:
+// make_view(value, 0, 0) produces the same u128 as
view_u128.
+//
+// SAFETY: the enclosing `if len <= 12` branch establishes
+// that view_u128 is a valid inline ByteView. Its low 32
+// bits encode `len` (<= 12) and the next 12 bytes are the
+// value bytes the source array produced for this row, so
+// every required invariant of `append_inline_view` holds.
Review Comment:
💯
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] perf: bypass values.value(i) for inline strings in ArrowBytesViewMap [datafusion]
alamb commented on PR #22172: URL: https://github.com/apache/datafusion/pull/22172#issuecomment-4460571238 Will plan to merge this in the next day or two to give some others time to review if they want -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] perf: bypass values.value(i) for inline strings in ArrowBytesViewMap [datafusion]
alamb commented on code in PR #22172:
URL: https://github.com/apache/datafusion/pull/22172#discussion_r3248121553
##
datafusion/physical-expr-common/src/binary_view_map.rs:
##
@@ -339,11 +339,31 @@ where
payload
} else {
// no existing value, make a new one
-let value: &[u8] = values.value(i).as_ref();
-let payload = make_payload_fn(Some(value));
+let (new_view, payload) = if len <= 12 {
+// Inline path: bytes are already packed in view_u128.
+// The inline ByteView format is [len:u32 LE][data:12
bytes zero-padded],
+// so extracting bytes from the u128 avoids a round-trip
through
+// values.value(i) (which reads the views buffer and
returns the same slice).
+let view_bytes = view_u128.to_le_bytes();
+let value = &view_bytes[4..4 + len as usize];
+let payload = make_payload_fn(Some(value));
+// For inline strings, the stored view is identical to the
input view:
+// make_view(value, 0, 0) produces the same u128 as
view_u128.
+//
+// SAFETY: the enclosing `if len <= 12` branch establishes
+// that view_u128 is a valid inline ByteView. Its low 32
+// bits encode `len` (<= 12) and the next 12 bytes are the
+// value bytes the source array produced for this row, so
+// every required invariant of `append_inline_view` holds.
Review Comment:
this seems overly verbose -- I think the justification is that `view_u128`
was a valid view and len <= 12
```suggestion
// SAFETY: view_u128 was a valid view, and the
encosing`len <= 12`
// ensures it is inline
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] perf: bypass values.value(i) for inline strings in ArrowBytesViewMap [datafusion]
alamb commented on PR #22172: URL: https://github.com/apache/datafusion/pull/22172#issuecomment-4459683619 > (I am trying to see if the last run was noise) The run was noise -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] perf: bypass values.value(i) for inline strings in ArrowBytesViewMap [datafusion]
RyanJamesStewart commented on PR #22172: URL: https://github.com/apache/datafusion/pull/22172#issuecomment-4455314950 v2 pushed (72bea57): 1. `append_inline_view` is now `unsafe fn` with a `# Safety` paragraph spelling out what goes wrong if a non-inline view is passed (downstream `views` consumers interpret the high 96 bits as `[buffer_index, offset]` into the completed/in_progress buffers, which is unsound). The single call site in `insert_if_new_inner` is annotated with a `SAFETY:` comment pointing at the enclosing `if len <= 12` branch that establishes the invariant. 2. `benches/binary_view_map_insert.rs` removed along with the `[[bench]]` entry in the crate's Cargo.toml. Behavior unchanged. 8/8 binary_view_map tests pass; clippy + fmt clean. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] perf: bypass values.value(i) for inline strings in ArrowBytesViewMap [datafusion]
adriangbot commented on PR #22172: URL: https://github.com/apache/datafusion/pull/22172#issuecomment-4454658602 🤖 Benchmark completed (GKE) | [trigger](https://github.com/apache/datafusion/pull/22172#issuecomment-4454535344) **Instance:** `c4a-highmem-16` (12 vCPU / 65 GiB) CPU Details (lscpu) ``` Architecture:aarch64 CPU op-mode(s): 64-bit Byte Order: Little Endian CPU(s): 16 On-line CPU(s) list: 0-15 Vendor ID: ARM Model name: Neoverse-V2 Model: 1 Thread(s) per core: 1 Core(s) per cluster: 16 Socket(s): - Cluster(s): 1 Stepping:r0p1 BogoMIPS:2000.00 Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti L1d cache: 1 MiB (16 instances) L1i cache: 1 MiB (16 instances) L2 cache:32 MiB (16 instances) L3 cache:80 MiB (1 instance) NUMA node(s):1 NUMA node0 CPU(s): 0-15 Vulnerability Gather data sampling: Not affected Vulnerability Indirect target selection: Not affected Vulnerability Itlb multihit: Not affected Vulnerability L1tf: Not affected Vulnerability Mds: Not affected Vulnerability Meltdown: Not affected Vulnerability Mmio stale data: Not affected Vulnerability Reg file data sampling:Not affected Vulnerability Retbleed: Not affected Vulnerability Spec rstack overflow: Not affected Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl Vulnerability Spectre v1:Mitigation; __user pointer sanitization Vulnerability Spectre v2:Mitigation; CSV2, BHB Vulnerability Srbds: Not affected Vulnerability Tsa: Not affected Vulnerability Tsx async abort: Not affected Vulnerability Vmscape: Not affected ``` Details ``` Comparing HEAD and perf_binary-view-map-inline-bypass Benchmark clickbench_partitioned.json ┏━━━┳━━━┳━━━┳━━━┓ ┃ Query ┃ HEAD ┃ perf_binary-view-map-inline-bypass ┃Change ┃ ┡━━━╇━━━╇━━━╇━━━┩ │ QQuery 0 │ 1.20 / 4.70 ±6.85 / 18.39 ms │ 1.22 / 4.67 ±6.81 / 18.29 ms │ no change │ │ QQuery 1 │12.68 / 12.86 ±0.12 / 13.00 ms │12.82 / 13.08 ±0.22 / 13.46 ms │ no change │ │ QQuery 2 │35.80 / 36.36 ±0.46 / 37.18 ms │35.45 / 35.90 ±0.40 / 36.64 ms │ no change │ │ QQuery 3 │31.42 / 32.07 ±0.55 / 33.08 ms │31.44 / 32.42 ±0.90 / 33.95 ms │ no change │ │ QQuery 4 │ 236.22 / 243.19 ±4.08 / 248.68 ms │ 235.46 / 239.66 ±2.23 / 241.30 ms │ no change │ │ QQuery 5 │ 287.36 / 288.95 ±2.41 / 293.71 ms │ 279.62 / 283.44 ±4.41 / 291.78 ms │ no change │ │ QQuery 6 │ 6.41 / 7.41 ±0.59 / 7.96 ms │ 5.96 / 6.90 ±0.61 / 7.79 ms │ +1.08x faster │ │ QQuery 7 │14.06 / 14.22 ±0.10 / 14.37 ms │13.79 / 15.71 ±3.50 / 22.70 ms │ 1.10x slower │ │ QQuery 8 │ 320.99 / 323.09 ±1.79 / 326.36 ms │ 320.63 / 323.46 ±2.26 / 326.96 ms │ no change │ │ QQuery 9 │ 459.62 / 466.85 ±6.91 / 479.98 ms │ 461.58 / 466.24 ±5.18 / 474.73 ms │ no change │ │ QQuery 10 │69.50 / 70.11 ±0.54 / 71.09 ms │70.31 / 71.21 ±0.79 / 72.64 ms │ no change │ │ QQuery 11 │79.64 / 80.67 ±0.67 / 81.34 ms │81.91 / 82.49 ±0.55 / 83.54 ms │ no change │ │ QQuery 12 │ 274.33 / 280.92 ±4.71 / 285.28 ms │ 277.33 / 284.95 ±7.39 / 297.65 ms │ no change │ │ QQuery 13 │ 386.74 / 392.53 ±4.19 / 399.77 ms │385.19 / 401.17 ±13.21 / 423.85 ms │ no change │ │ QQuery 14 │ 282.37 / 288.07 ±5.31 / 297.42 ms │ 280.14 / 284.57 ±3.09 / 289.25 ms │ no change │
Re: [PR] perf: bypass values.value(i) for inline strings in ArrowBytesViewMap [datafusion]
adriangbot commented on PR #22172: URL: https://github.com/apache/datafusion/pull/22172#issuecomment-4454557211 🤖 Benchmark running (GKE) | [trigger](https://github.com/apache/datafusion/pull/22172#issuecomment-4454535344) **Instance:** `c4a-highmem-16` (12 vCPU / 65 GiB) | `Linux bench-c4454535344-108-f4fng 6.12.68+ #1 SMP Wed Apr 1 02:23:28 UTC 2026 aarch64 GNU/Linux` CPU Details (lscpu) ``` Architecture:aarch64 CPU op-mode(s): 64-bit Byte Order: Little Endian CPU(s): 16 On-line CPU(s) list: 0-15 Vendor ID: ARM Model name: Neoverse-V2 Model: 1 Thread(s) per core: 1 Core(s) per cluster: 16 Socket(s): - Cluster(s): 1 Stepping:r0p1 BogoMIPS:2000.00 Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti L1d cache: 1 MiB (16 instances) L1i cache: 1 MiB (16 instances) L2 cache:32 MiB (16 instances) L3 cache:80 MiB (1 instance) NUMA node(s):1 NUMA node0 CPU(s): 0-15 Vulnerability Gather data sampling: Not affected Vulnerability Indirect target selection: Not affected Vulnerability Itlb multihit: Not affected Vulnerability L1tf: Not affected Vulnerability Mds: Not affected Vulnerability Meltdown: Not affected Vulnerability Mmio stale data: Not affected Vulnerability Reg file data sampling:Not affected Vulnerability Retbleed: Not affected Vulnerability Spec rstack overflow: Not affected Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl Vulnerability Spectre v1:Mitigation; __user pointer sanitization Vulnerability Spectre v2:Mitigation; CSV2, BHB Vulnerability Srbds: Not affected Vulnerability Tsa: Not affected Vulnerability Tsx async abort: Not affected Vulnerability Vmscape: Not affected ``` Comparing perf/binary-view-map-inline-bypass (1364643d693adecc4c1b3ff7c4edbdb2adf70346) to 18a219c (merge-base) [diff](https://github.com/apache/datafusion/compare/18a219c7285278d90c91d6ed51ee96a43e6a35e6..1364643d693adecc4c1b3ff7c4edbdb2adf70346) using: clickbench_partitioned Results will be posted here when complete --- [File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] perf: bypass values.value(i) for inline strings in ArrowBytesViewMap [datafusion]
alamb commented on PR #22172: URL: https://github.com/apache/datafusion/pull/22172#issuecomment-4454535344 run benchmark clickbench_partitioned -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] perf: bypass values.value(i) for inline strings in ArrowBytesViewMap [datafusion]
alamb commented on PR #22172: URL: https://github.com/apache/datafusion/pull/22172#issuecomment-4454536355 (I am trying to see if the last run was noise) -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] perf: bypass values.value(i) for inline strings in ArrowBytesViewMap [datafusion]
adriangbot commented on PR #22172: URL: https://github.com/apache/datafusion/pull/22172#issuecomment-4454153171 🤖 Benchmark completed (GKE) | [trigger](https://github.com/apache/datafusion/pull/22172#issuecomment-4453991269) **Instance:** `c4a-highmem-16` (12 vCPU / 65 GiB) CPU Details (lscpu) ``` Architecture:aarch64 CPU op-mode(s): 64-bit Byte Order: Little Endian CPU(s): 16 On-line CPU(s) list: 0-15 Vendor ID: ARM Model name: Neoverse-V2 Model: 1 Thread(s) per core: 1 Core(s) per cluster: 16 Socket(s): - Cluster(s): 1 Stepping:r0p1 BogoMIPS:2000.00 Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti L1d cache: 1 MiB (16 instances) L1i cache: 1 MiB (16 instances) L2 cache:32 MiB (16 instances) L3 cache:80 MiB (1 instance) NUMA node(s):1 NUMA node0 CPU(s): 0-15 Vulnerability Gather data sampling: Not affected Vulnerability Indirect target selection: Not affected Vulnerability Itlb multihit: Not affected Vulnerability L1tf: Not affected Vulnerability Mds: Not affected Vulnerability Meltdown: Not affected Vulnerability Mmio stale data: Not affected Vulnerability Reg file data sampling:Not affected Vulnerability Retbleed: Not affected Vulnerability Spec rstack overflow: Not affected Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl Vulnerability Spectre v1:Mitigation; __user pointer sanitization Vulnerability Spectre v2:Mitigation; CSV2, BHB Vulnerability Srbds: Not affected Vulnerability Tsa: Not affected Vulnerability Tsx async abort: Not affected Vulnerability Vmscape: Not affected ``` Details ``` Comparing HEAD and perf_binary-view-map-inline-bypass Benchmark clickbench_partitioned.json ┏━━━┳━━━┳━━━┳━━━┓ ┃ Query ┃ HEAD ┃ perf_binary-view-map-inline-bypass ┃Change ┃ ┡━━━╇━━━╇━━━╇━━━┩ │ QQuery 0 │ 1.21 / 4.54 ±6.61 / 17.76 ms │ 1.22 / 4.58 ±6.66 / 17.89 ms │ no change │ │ QQuery 1 │12.51 / 12.66 ±0.18 / 13.02 ms │12.67 / 12.74 ±0.06 / 12.85 ms │ no change │ │ QQuery 2 │35.12 / 35.69 ±0.51 / 36.50 ms │35.20 / 35.41 ±0.19 / 35.71 ms │ no change │ │ QQuery 3 │30.72 / 31.19 ±0.65 / 32.46 ms │30.64 / 30.84 ±0.11 / 30.97 ms │ no change │ │ QQuery 4 │ 226.22 / 233.31 ±6.51 / 243.59 ms │ 227.15 / 232.17 ±3.60 / 237.80 ms │ no change │ │ QQuery 5 │ 275.41 / 277.22 ±1.70 / 280.11 ms │ 274.05 / 275.30 ±1.51 / 278.06 ms │ no change │ │ QQuery 6 │ 6.76 / 7.49 ±0.38 / 7.88 ms │ 6.55 / 7.26 ±0.58 / 8.14 ms │ no change │ │ QQuery 7 │13.37 / 13.53 ±0.09 / 13.63 ms │13.40 / 13.59 ±0.12 / 13.77 ms │ no change │ │ QQuery 8 │ 311.36 / 317.82 ±4.32 / 324.55 ms │ 312.10 / 315.05 ±2.05 / 317.63 ms │ no change │ │ QQuery 9 │428.67 / 447.16 ±10.90 / 462.58 ms │ 435.22 / 444.52 ±5.16 / 448.92 ms │ no change │ │ QQuery 10 │67.86 / 69.08 ±0.92 / 70.22 ms │70.02 / 73.65 ±5.19 / 83.66 ms │ 1.07x slower │ │ QQuery 11 │78.26 / 79.96 ±1.78 / 83.38 ms │80.70 / 81.69 ±0.73 / 82.73 ms │ no change │ │ QQuery 12 │ 268.33 / 273.33 ±5.10 / 282.45 ms │ 267.56 / 272.79 ±4.04 / 279.39 ms │ no change │ │ QQuery 13 │ 376.79 / 383.35 ±5.05 / 389.23 ms │ 376.74 / 382.83 ±4.27 / 389.45 ms │ no change │ │ QQuery 14 │ 276.50 / 279.56 ±2.85 / 284.49 ms │ 279.15 / 282.42 ±3.04 / 287.34 ms │ no change │
Re: [PR] perf: bypass values.value(i) for inline strings in ArrowBytesViewMap [datafusion]
adriangbot commented on PR #22172: URL: https://github.com/apache/datafusion/pull/22172#issuecomment-4454128039 🤖 Benchmark completed (GKE) | [trigger](https://github.com/apache/datafusion/pull/22172#issuecomment-4453991269) **Instance:** `c4a-highmem-16` (12 vCPU / 65 GiB) CPU Details (lscpu) ``` Architecture:aarch64 CPU op-mode(s): 64-bit Byte Order: Little Endian CPU(s): 16 On-line CPU(s) list: 0-15 Vendor ID: ARM Model name: Neoverse-V2 Model: 1 Thread(s) per core: 1 Core(s) per cluster: 16 Socket(s): - Cluster(s): 1 Stepping:r0p1 BogoMIPS:2000.00 Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti L1d cache: 1 MiB (16 instances) L1i cache: 1 MiB (16 instances) L2 cache:32 MiB (16 instances) L3 cache:80 MiB (1 instance) NUMA node(s):1 NUMA node0 CPU(s): 0-15 Vulnerability Gather data sampling: Not affected Vulnerability Indirect target selection: Not affected Vulnerability Itlb multihit: Not affected Vulnerability L1tf: Not affected Vulnerability Mds: Not affected Vulnerability Meltdown: Not affected Vulnerability Mmio stale data: Not affected Vulnerability Reg file data sampling:Not affected Vulnerability Retbleed: Not affected Vulnerability Spec rstack overflow: Not affected Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl Vulnerability Spectre v1:Mitigation; __user pointer sanitization Vulnerability Spectre v2:Mitigation; CSV2, BHB Vulnerability Srbds: Not affected Vulnerability Tsa: Not affected Vulnerability Tsx async abort: Not affected Vulnerability Vmscape: Not affected ``` Details ``` Comparing HEAD and perf_binary-view-map-inline-bypass Benchmark tpcds_sf1.json ┏━━━┳━━━┳━━━┳━━┓ ┃ Query ┃ HEAD ┃ perf_binary-view-map-inline-bypass ┃ Change ┃ ┡━━━╇━━━╇━━━╇━━┩ │ QQuery 1 │ 6.43 / 7.18 ±0.89 / 8.82 ms │ 6.43 / 6.90 ±0.87 / 8.65 ms │no change │ │ QQuery 2 │82.04 / 82.39 ±0.20 / 82.67 ms │82.33 / 82.57 ±0.27 / 82.94 ms │no change │ │ QQuery 3 │29.20 / 29.44 ±0.24 / 29.81 ms │28.87 / 29.23 ±0.21 / 29.46 ms │no change │ │ QQuery 4 │ 532.84 / 536.24 ±3.80 / 543.44 ms │ 524.40 / 531.46 ±5.64 / 538.58 ms │no change │ │ QQuery 5 │52.93 / 53.29 ±0.29 / 53.67 ms │52.82 / 53.36 ±0.38 / 53.97 ms │no change │ │ QQuery 6 │36.11 / 36.55 ±0.25 / 36.85 ms │35.59 / 35.93 ±0.34 / 36.51 ms │no change │ │ QQuery 7 │ 111.33 / 113.24 ±1.81 / 116.49 ms │ 111.03 / 113.57 ±3.28 / 119.99 ms │no change │ │ QQuery 8 │39.48 / 39.93 ±0.23 / 40.11 ms │40.06 / 40.35 ±0.27 / 40.71 ms │no change │ │ QQuery 9 │53.47 / 56.22 ±1.80 / 58.48 ms │54.64 / 56.53 ±1.24 / 58.56 ms │no change │ │ QQuery 10 │81.15 / 82.17 ±1.21 / 84.40 ms │82.42 / 83.73 ±1.50 / 86.46 ms │no change │ │ QQuery 11 │ 325.31 / 329.38 ±4.24 / 337.07 ms │ 328.19 / 331.92 ±3.88 / 339.10 ms │no change │ │ QQuery 12 │29.31 / 29.58 ±0.22 / 29.96 ms │29.07 / 29.53 ±0.30 / 29.85 ms │no change │ │ QQuery 13 │ 129.31 / 129.95 ±0.47 / 130.76 ms │ 129.93 / 130.24 ±0.17 / 130.43 ms │no change │ │ QQuery 14 │ 519.35 / 521.56 ±1.97 / 525.24 ms │ 518.44 / 519.17 ±0.46 / 519.88 ms │no change │ │ QQuery 15 │62.89 / 64.51 ±1.83 / 68.08 ms │62.52 / 62.94 ±0.39 / 63.53 ms │no change │ │ QQuery 16 │ 7.1
Re: [PR] perf: bypass values.value(i) for inline strings in ArrowBytesViewMap [datafusion]
adriangbot commented on PR #22172: URL: https://github.com/apache/datafusion/pull/22172#issuecomment-4454120283 🤖 Benchmark completed (GKE) | [trigger](https://github.com/apache/datafusion/pull/22172#issuecomment-4453991269) **Instance:** `c4a-highmem-16` (12 vCPU / 65 GiB) CPU Details (lscpu) ``` Architecture:aarch64 CPU op-mode(s): 64-bit Byte Order: Little Endian CPU(s): 16 On-line CPU(s) list: 0-15 Vendor ID: ARM Model name: Neoverse-V2 Model: 1 Thread(s) per core: 1 Core(s) per cluster: 16 Socket(s): - Cluster(s): 1 Stepping:r0p1 BogoMIPS:2000.00 Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti L1d cache: 1 MiB (16 instances) L1i cache: 1 MiB (16 instances) L2 cache:32 MiB (16 instances) L3 cache:80 MiB (1 instance) NUMA node(s):1 NUMA node0 CPU(s): 0-15 Vulnerability Gather data sampling: Not affected Vulnerability Indirect target selection: Not affected Vulnerability Itlb multihit: Not affected Vulnerability L1tf: Not affected Vulnerability Mds: Not affected Vulnerability Meltdown: Not affected Vulnerability Mmio stale data: Not affected Vulnerability Reg file data sampling:Not affected Vulnerability Retbleed: Not affected Vulnerability Spec rstack overflow: Not affected Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl Vulnerability Spectre v1:Mitigation; __user pointer sanitization Vulnerability Spectre v2:Mitigation; CSV2, BHB Vulnerability Srbds: Not affected Vulnerability Tsa: Not affected Vulnerability Tsx async abort: Not affected Vulnerability Vmscape: Not affected ``` Details ``` Comparing HEAD and perf_binary-view-map-inline-bypass Benchmark tpch_sf1.json ┏━━━┳┳┳━━━┓ ┃ Query ┃ HEAD ┃ perf_binary-view-map-inline-bypass ┃Change ┃ ┡━━━╇╇╇━━━┩ │ QQuery 1 │ 38.27 / 39.31 ±1.18 / 41.37 ms │ 38.19 / 39.05 ±1.11 / 41.06 ms │ no change │ │ QQuery 2 │ 20.40 / 20.54 ±0.13 / 20.76 ms │ 20.68 / 21.21 ±0.44 / 21.92 ms │ no change │ │ QQuery 3 │ 34.98 / 37.17 ±1.14 / 38.18 ms │ 34.32 / 36.82 ±1.27 / 37.86 ms │ no change │ │ QQuery 4 │ 16.95 / 17.38 ±0.59 / 18.54 ms │ 16.97 / 17.18 ±0.14 / 17.40 ms │ no change │ │ QQuery 5 │ 45.62 / 48.21 ±1.47 / 49.65 ms │ 42.08 / 45.84 ±2.62 / 49.78 ms │ no change │ │ QQuery 6 │ 16.17 / 16.55 ±0.55 / 17.63 ms │ 16.06 / 16.56 ±0.48 / 17.44 ms │ no change │ │ QQuery 7 │ 49.53 / 51.17 ±2.17 / 55.27 ms │ 48.25 / 49.66 ±1.10 / 51.64 ms │ no change │ │ QQuery 8 │ 44.62 / 44.96 ±0.21 / 45.22 ms │ 44.79 / 45.94 ±0.93 / 47.46 ms │ no change │ │ QQuery 9 │ 49.88 / 51.50 ±1.56 / 53.96 ms │ 50.55 / 51.39 ±0.77 / 52.41 ms │ no change │ │ QQuery 10 │ 63.45 / 65.43 ±1.19 / 66.92 ms │ 63.70 / 64.96 ±1.52 / 67.37 ms │ no change │ │ QQuery 11 │ 13.51 / 14.40 ±0.70 / 15.38 ms │ 13.61 / 13.81 ±0.19 / 14.10 ms │ no change │ │ QQuery 12 │ 24.75 / 25.26 ±0.73 / 26.71 ms │ 24.45 / 24.90 ±0.25 / 25.22 ms │ no change │ │ QQuery 13 │ 35.16 / 35.77 ±0.48 / 36.34 ms │ 34.66 / 35.86 ±0.66 / 36.53 ms │ no change │ │ QQuery 14 │ 25.27 / 25.56 ±0.21 / 25.92 ms │ 25.18 / 25.65 ±0.67 / 26.97 ms │ no change │ │ QQuery 15 │ 30.58 / 31.45 ±1.33 / 34.11 ms │ 30.66 / 31.06 ±0.47 / 31.93 ms │ no change │ │ QQuery 16 │ 15.44 / 15.66 ±0.19 / 15.98 ms │ 15.79 / 16.00 ±0.17 / 16.25 ms │ no change │ │ QQuery 17 │ 78.63 / 82.23 ±3.96 / 89.86 ms │ 74.57 / 77.79 ±3.77 / 83.46 ms │ +
Re: [PR] perf: bypass values.value(i) for inline strings in ArrowBytesViewMap [datafusion]
adriangbot commented on PR #22172: URL: https://github.com/apache/datafusion/pull/22172#issuecomment-4454013101 🤖 Benchmark running (GKE) | [trigger](https://github.com/apache/datafusion/pull/22172#issuecomment-4453991269) **Instance:** `c4a-highmem-16` (12 vCPU / 65 GiB) | `Linux bench-c4453991269-104-fn27b 6.12.68+ #1 SMP Wed Apr 1 02:23:28 UTC 2026 aarch64 GNU/Linux` CPU Details (lscpu) ``` Architecture:aarch64 CPU op-mode(s): 64-bit Byte Order: Little Endian CPU(s): 16 On-line CPU(s) list: 0-15 Vendor ID: ARM Model name: Neoverse-V2 Model: 1 Thread(s) per core: 1 Core(s) per cluster: 16 Socket(s): - Cluster(s): 1 Stepping:r0p1 BogoMIPS:2000.00 Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti L1d cache: 1 MiB (16 instances) L1i cache: 1 MiB (16 instances) L2 cache:32 MiB (16 instances) L3 cache:80 MiB (1 instance) NUMA node(s):1 NUMA node0 CPU(s): 0-15 Vulnerability Gather data sampling: Not affected Vulnerability Indirect target selection: Not affected Vulnerability Itlb multihit: Not affected Vulnerability L1tf: Not affected Vulnerability Mds: Not affected Vulnerability Meltdown: Not affected Vulnerability Mmio stale data: Not affected Vulnerability Reg file data sampling:Not affected Vulnerability Retbleed: Not affected Vulnerability Spec rstack overflow: Not affected Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl Vulnerability Spectre v1:Mitigation; __user pointer sanitization Vulnerability Spectre v2:Mitigation; CSV2, BHB Vulnerability Srbds: Not affected Vulnerability Tsa: Not affected Vulnerability Tsx async abort: Not affected Vulnerability Vmscape: Not affected ``` Comparing perf/binary-view-map-inline-bypass (1364643d693adecc4c1b3ff7c4edbdb2adf70346) to 18a219c (merge-base) [diff](https://github.com/apache/datafusion/compare/18a219c7285278d90c91d6ed51ee96a43e6a35e6..1364643d693adecc4c1b3ff7c4edbdb2adf70346) using: clickbench_partitioned Results will be posted here when complete --- [File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] perf: bypass values.value(i) for inline strings in ArrowBytesViewMap [datafusion]
adriangbot commented on PR #22172: URL: https://github.com/apache/datafusion/pull/22172#issuecomment-4454013293 🤖 Benchmark running (GKE) | [trigger](https://github.com/apache/datafusion/pull/22172#issuecomment-4453991269) **Instance:** `c4a-highmem-16` (12 vCPU / 65 GiB) | `Linux bench-c4453991269-106-6fmpv 6.12.68+ #1 SMP Wed Apr 1 02:23:28 UTC 2026 aarch64 GNU/Linux` CPU Details (lscpu) ``` Architecture:aarch64 CPU op-mode(s): 64-bit Byte Order: Little Endian CPU(s): 16 On-line CPU(s) list: 0-15 Vendor ID: ARM Model name: Neoverse-V2 Model: 1 Thread(s) per core: 1 Core(s) per cluster: 16 Socket(s): - Cluster(s): 1 Stepping:r0p1 BogoMIPS:2000.00 Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti L1d cache: 1 MiB (16 instances) L1i cache: 1 MiB (16 instances) L2 cache:32 MiB (16 instances) L3 cache:80 MiB (1 instance) NUMA node(s):1 NUMA node0 CPU(s): 0-15 Vulnerability Gather data sampling: Not affected Vulnerability Indirect target selection: Not affected Vulnerability Itlb multihit: Not affected Vulnerability L1tf: Not affected Vulnerability Mds: Not affected Vulnerability Meltdown: Not affected Vulnerability Mmio stale data: Not affected Vulnerability Reg file data sampling:Not affected Vulnerability Retbleed: Not affected Vulnerability Spec rstack overflow: Not affected Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl Vulnerability Spectre v1:Mitigation; __user pointer sanitization Vulnerability Spectre v2:Mitigation; CSV2, BHB Vulnerability Srbds: Not affected Vulnerability Tsa: Not affected Vulnerability Tsx async abort: Not affected Vulnerability Vmscape: Not affected ``` Comparing perf/binary-view-map-inline-bypass (1364643d693adecc4c1b3ff7c4edbdb2adf70346) to 18a219c (merge-base) [diff](https://github.com/apache/datafusion/compare/18a219c7285278d90c91d6ed51ee96a43e6a35e6..1364643d693adecc4c1b3ff7c4edbdb2adf70346) using: tpch Results will be posted here when complete --- [File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] perf: bypass values.value(i) for inline strings in ArrowBytesViewMap [datafusion]
adriangbot commented on PR #22172: URL: https://github.com/apache/datafusion/pull/22172#issuecomment-4454006078 🤖 Benchmark running (GKE) | [trigger](https://github.com/apache/datafusion/pull/22172#issuecomment-4453991269) **Instance:** `c4a-highmem-16` (12 vCPU / 65 GiB) | `Linux bench-c4453991269-105-h75vk 6.12.68+ #1 SMP Wed Apr 1 02:23:28 UTC 2026 aarch64 GNU/Linux` CPU Details (lscpu) ``` Architecture:aarch64 CPU op-mode(s): 64-bit Byte Order: Little Endian CPU(s): 16 On-line CPU(s) list: 0-15 Vendor ID: ARM Model name: Neoverse-V2 Model: 1 Thread(s) per core: 1 Core(s) per cluster: 16 Socket(s): - Cluster(s): 1 Stepping:r0p1 BogoMIPS:2000.00 Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti L1d cache: 1 MiB (16 instances) L1i cache: 1 MiB (16 instances) L2 cache:32 MiB (16 instances) L3 cache:80 MiB (1 instance) NUMA node(s):1 NUMA node0 CPU(s): 0-15 Vulnerability Gather data sampling: Not affected Vulnerability Indirect target selection: Not affected Vulnerability Itlb multihit: Not affected Vulnerability L1tf: Not affected Vulnerability Mds: Not affected Vulnerability Meltdown: Not affected Vulnerability Mmio stale data: Not affected Vulnerability Reg file data sampling:Not affected Vulnerability Retbleed: Not affected Vulnerability Spec rstack overflow: Not affected Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl Vulnerability Spectre v1:Mitigation; __user pointer sanitization Vulnerability Spectre v2:Mitigation; CSV2, BHB Vulnerability Srbds: Not affected Vulnerability Tsa: Not affected Vulnerability Tsx async abort: Not affected Vulnerability Vmscape: Not affected ``` Comparing perf/binary-view-map-inline-bypass (1364643d693adecc4c1b3ff7c4edbdb2adf70346) to 18a219c (merge-base) [diff](https://github.com/apache/datafusion/compare/18a219c7285278d90c91d6ed51ee96a43e6a35e6..1364643d693adecc4c1b3ff7c4edbdb2adf70346) using: tpcds Results will be posted here when complete --- [File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] perf: bypass values.value(i) for inline strings in ArrowBytesViewMap [datafusion]
alamb commented on PR #22172: URL: https://github.com/apache/datafusion/pull/22172#issuecomment-4453991269 run benchmarks -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] perf: bypass values.value(i) for inline strings in ArrowBytesViewMap [datafusion]
alamb commented on PR #22172: URL: https://github.com/apache/datafusion/pull/22172#issuecomment-4453990830 FYI @Dandandan -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] perf: bypass values.value(i) for inline strings in ArrowBytesViewMap [datafusion]
alamb commented on code in PR #22172:
URL: https://github.com/apache/datafusion/pull/22172#discussion_r3243710520
##
datafusion/physical-expr-common/src/binary_view_map.rs:
##
@@ -389,6 +403,17 @@ where
}
}
+/// Append an already-computed inline view (len <= 12) directly, bypassing
+/// buffer allocation. The caller guarantees that `view` is a valid inline
+/// ByteView (length field in the low 32 bits is <= 12).
+///
+/// Returns the view that was stored (identical to the argument).
+fn append_inline_view(&mut self, view: u128) -> u128 {
Review Comment:
Given this assumption I think this function should be marked as `unsafe` (I
think it is correct, but the compiler can't verify it)
```suggestion
unsafe fn append_inline_view(&mut self, view: u128) -> u128 {
```
Then this will force us to annotate the callsite with `unsafe` as well where
it can be more easily verified that the input was a valid inline view so this
function is safe
##
datafusion/physical-expr-common/benches/binary_view_map_insert.rs:
##
@@ -0,0 +1,91 @@
+// Licensed to the Apache Software Foundation (ASF) under one
Review Comment:
While this benchmark is cool, I am not sure we need to commit it to the repo
as I think its utility will be low after this PR has been merged (I think it is
unlikely to get run regularly)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
