sunchao commented on code in PR #11279:
URL: https://github.com/apache/arrow-rs/pull/11279#discussion_r4170834321
##########
arrow-select/src/interleave.rs:
##########
@@ -125,6 +128,89 @@ pub fn interleave(
}
}
+/// Interleaves byte view arrays into independently owned, compact buffers.
Review Comment:
I switched this to
`Interleaver::new().with_compact_byte_views(true).interleave(...)` and removed
the additional public free function. An optional
`with_preserve_byte_view_sharing(true)` copies a repeated source byte range
once into the output; it is separate because tracking ranges has a cost. These
options apply to top-level StringView/BinaryView arrays. The configurable
record-batch method applies the same options column by column.
On current head `c8ee97880e46af9257a41e8df355bd9bcf23f238`, the original
free `interleave` function body matches exact base
`705e6405b56cc01efb1c267203d240d0d85804ab` and has no executable reference to
compact kernels. The configurable method delegates to it for defaults and other
types. Disassembly while investigating the controls showed two additional
outlined setup calls. After restoring local validation and adding an inlining
hint to `Interleave::new`, those calls are absent in the measured build. That
is not a general compiler or zero-overhead guarantee.
One integer-only client calling the original free API grew from 2,741,056 to
2,742,800 stripped bytes (+1,744, 0.064%) on exact base versus this head: Rust
1.99.0, optimization level 3, one client codegen unit, LTO off, matching
dependency sources/features. Both clients checked identical output. This is a
narrow example, not a guarantee for other clients or link settings.
The description and reproduction comments contain fresh
exact-base/current-head timing and allocation evidence, including adverse
cases. I kept the PR in draft while we settle the API; this thread remains open
for design acceptance.
<details>
<summary>Reproduce the narrow original-free-API code-size
comparison</summary>
This is one nullable-Int32 caller of the original free `interleave` API,
not a general application-size or runtime claim. The matched final run
measured
2,741,056 stripped bytes at base and 2,742,800 at head (+1,744; +0.064%).
Absolute paths, target platform, and linker layout can affect exact sizes.
After the timing reproduction has built the exact revisions, run the
following
from the directory containing `arrow-base/`, `arrow-head/`,
`timing-results/build-base.jsonl`, and `timing-results/build-head.jsonl`.
These must be the local Cargo
JSON build records for those checkouts, not logs copied from another machine.
This only links two small clients against the existing release dependencies.
```bash
set -eu
mkdir -p code-size-repro
cat > code-size-repro/client.rs <<'RS'
use std::hint::black_box;
use arrow_array::{Array, Int32Array};
use arrow_select::interleave::interleave;
fn main() -> Result<(), Box<dyn std::error::Error>> {
let left = Int32Array::from(vec![Some(10), None, Some(30), Some(40)]);
let right = Int32Array::from(vec![-7, -8, -9]);
let arrays: [&dyn Array; 2] = [&left, &right];
let indices = [(1, 0), (0, 2), (0, 1), (1, 2), (0, 0), (0, 2)];
let result = interleave(black_box(&arrays), black_box(&indices))?;
let result = result.as_any().downcast_ref::<Int32Array>().unwrap();
let actual: Vec<Option<i32>> = result.iter().collect();
assert_eq!(actual, [Some(-7), Some(30), None, Some(-9), Some(10),
Some(30)]);
println!("{:?}", black_box(actual));
Ok(())
}
RS
python3 - <<'PY'
import hashlib
import json
import pathlib
import shlex
import subprocess
root = pathlib.Path.cwd()
out = root / "code-size-repro"
base = root / "arrow-base"
head = root / "arrow-head"
kernel = "705e6405b56cc01efb1c267203d240d0d85804ab"
base_sha = "0b317f8a56949f4872bfdf4826e779ce5ff9aaf1"
head_sha = "c8ee97880e46af9257a41e8df355bd9bcf23f238"
def check(condition, message):
if not condition:
raise RuntimeError(message)
def git(checkout, *args):
return subprocess.check_output(
["git", "-C", str(checkout), *args], text=True
).strip()
def digest(path):
value = hashlib.sha256(path.read_bytes()).hexdigest()
print(f"sha256 {value} {path}")
return value
for checkout, revision in [(base, base_sha), (head, head_sha)]:
check(git(checkout, "rev-parse", "HEAD") == revision, "Wrong revision")
check(not git(checkout, "status", "--porcelain"), "Dirty checkout")
check(git(checkout, "merge-base", kernel, revision) == kernel,
"Wrong kernel ancestry")
# The companion changes benchmarks only; the PR also changes
interleave.rs.
excludes = [
":!arrow/benches/interleave_kernels.rs",
":!arrow/benches/interleave_byte_view_cases/mod.rs",
]
if checkout == head:
excludes.append(":!arrow-select/src/interleave.rs")
git(checkout, "diff", "--exit-code", kernel, revision, "--", ".",
*excludes)
print(f"source {checkout.name} {revision} tree={git(checkout,
'rev-parse', 'HEAD^{tree}')}")
check((base / "Cargo.lock").read_bytes() == (head /
"Cargo.lock").read_bytes(),
"Cargo locks differ")
client = out / "client.rs"
check(digest(client) ==
"a305041224c2cd1bbe5f78dceef0833ed7a93869f0ab9fb4b5ce1f44fabc47d1",
"Client source differs")
digest(base / "Cargo.lock")
version = subprocess.check_output(["rustc", "+1.99.0", "-V"],
text=True).strip()
check(version == "rustc 1.99.0 (b940084d7 2026-09-28)", "Wrong Rust
compiler")
subprocess.run(["rustc", "+1.99.0", "-Vv"], check=True)
def artifacts(checkout, log):
# Select actual compiler artifacts by target and source manifest, not
globs.
records = [json.loads(line) for line in log.read_text().splitlines() if
line]
libs, fingerprints = {}, {}
profile = {"opt_level": "3", "debuginfo": 0, "debug_assertions": False,
"overflow_checks": False, "test": False}
for target, package in [("arrow_array", "arrow-array"),
("arrow_select", "arrow-select")]:
manifest = (checkout / package / "Cargo.toml").resolve()
matching = [r for r in records
if r.get("reason") == "compiler-artifact"
and r["target"]["name"] == target
and pathlib.Path(r["manifest_path"]).resolve() ==
manifest]
check(bool(matching), f"Missing {target} artifact in {log}")
for record in matching:
check(record["features"] == [] and record["profile"] == profile,
f"Unexpected release profile/features for {target}")
paths = {pathlib.Path(name).resolve() for r in matching
for name in r["filenames"] if name.endswith(".rlib")}
check(len(paths) == 1, f"Ambiguous {target} rlib")
lib = paths.pop()
deps = (checkout / "target/release/deps").resolve()
check(lib.is_file() and lib.parent == deps, "Wrong dependency
directory")
libs[target] = lib
# Derive the fingerprint identity from the selected artifact.
suffix = lib.stem.rsplit("-", 1)[1]
fingerprint = deps.parent / ".fingerprint" / f"{package}-{suffix}"
fp = json.loads((fingerprint / f"lib-{target}.json").read_text())
check(fp["features"] == "[]" and fp["rustflags"] == []
and fp["compile_kind"] == 0, "Unexpected dependency build
flags")
fingerprints[target] = fp
digest(lib)
digest(log)
return libs, fingerprints
base_libs, base_fp = artifacts(base, root /
"timing-results/build-base.jsonl")
head_libs, head_fp = artifacts(head, root /
"timing-results/build-head.jsonl")
check(base_fp == head_fp, "Dependency fingerprints/profile inputs differ")
outputs, binaries = [], []
for label, checkout, libs in [("base", base, base_libs), ("head", head,
head_libs)]:
binary = out / f"{label}-client"
command = [
"rustc", "+1.99.0", "--edition=2024", "--crate-name",
"interleave_client",
"-C", "opt-level=3", "-C", "debuginfo=0", "-C", "strip=symbols",
"-C", "codegen-units=1", "-C", "lto=off",
"--extern", f"arrow_array={libs['arrow_array']}",
"--extern", f"arrow_select={libs['arrow_select']}",
"-L", f"dependency={libs['arrow_select'].parent}",
str(client), "-o", str(binary),
]
print(shlex.join(command), flush=True)
subprocess.run(command, check=True)
outputs.append(subprocess.check_output([str(binary)], text=True))
binaries.append(binary)
digest(binary)
expected = "[Some(-7), Some(30), None, Some(-9), Some(10), Some(30)]\n"
check(outputs == [expected, expected], "Client outputs differ or are
incorrect")
sizes = [binary.stat().st_size for binary in binaries]
print(expected, end="")
print(f"base={sizes[0]} head={sizes[1]} delta={sizes[1] - sizes[0]} bytes "
f"({100 * (sizes[1] - sizes[0]) / sizes[0]:+.3f}%)")
subprocess.run(["size", *map(str, binaries)], check=True)
PY
```
Each client uses its own checkout's complete release dependency graph; no
base/head rlibs are mixed. Matching fingerprints verify compiler/profile/flag
inputs, not byte identity of separately compiled libraries. The checksum and
command output supplies local provenance.
GNU `size`'s text total includes read-only sections, and `bss` is not on-disk
payload. This experiment does not measure RSS, allocation peaks, compact API
code size, or runtime performance.
</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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
