From c3f2b0c80197d9361a543a17e6cbbf7540dc9499 Mon Sep 17 00:00:00 2001 From: tison Date: Fri, 30 Oct 2026 17:15:13 +0800 Subject: [PATCH 1/2] perf(xor): release construction graph before fingerprint allocation --- .github/workflows/ci.yml | 27 +++++++++ CHANGELOG.md | 2 +- CONTRIBUTING.md | 9 +++ datasketches/src/xor/filter.rs | 6 ++ .../tests/serde_tests/XorInterop.java | 48 ++++++++++++++++ tests-integration/tests/serde_tests/xor.rs | 56 +++++++++++++++++++ 6 files changed, 147 insertions(+), 1 deletion(-) create mode 100644 tests-integration/tests/serde_tests/XorInterop.java diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a83d451a..02842962 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -92,6 +92,31 @@ jobs: shell: bash run: cargo x test + xor-java: + name: XOR Java interoperability + runs-on: ubuntu-24.04 + steps: + - uses: actions/checkout@v7 + - uses: actions/checkout@v7 + with: + repository: apache/datasketches-java + ref: 1034fb33b6e013334ff870f130b9c388cae5dfa9 + path: interop-java + - uses: actions/setup-java@v6 + with: + distribution: temurin + java-version: '25' + - name: Compile Java reader and builder + working-directory: interop-java + run: mvn --batch-mode -DskipTests compile + - name: Install toolchain + run: rustup toolchain install stable && rustup default stable + - uses: Swatinem/rust-cache@v2 + - name: Check both serialization directions + env: + DATASKETCHES_JAVA_CLASSPATH: ${{ github.workspace }}/interop-java/target/classes + run: cargo test -p tests-integration --test serde_tests java_reads_rust_images -- --ignored + required: name: Required runs-on: ubuntu-24.04 @@ -99,12 +124,14 @@ jobs: needs: - check - test + - xor-java steps: - name: Guardian run: | if [[ ! ( \ "${{ needs.check.result }}" == "success" \ && "${{ needs.test.result }}" == "success" \ + && "${{ needs.xor-java.result }}" == "success" \ ) ]]; then echo "Required jobs haven't been completed successfully." exit 1 diff --git a/CHANGELOG.md b/CHANGELOG.md index 376478fd..cbc6000b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ All significant changes to this project will be documented in this file. ### New features -* Add immutable `XorFilter` and `XorFilterBuilder` behind the `xor` feature, with 8-, 16-, and 32-bit fingerprints, consuming builder configuration with complete-input construction and bounded seeded retries, optional cardinality metadata for imported filters, standard serialization for all three widths (Java-compatible for 8/16-bit filters), and raw parts for application-defined storage. +* Add immutable `XorFilter` and `XorFilterBuilder` behind the `xor` feature, with 8-, 16-, and 32-bit fingerprints, consuming builder configuration with complete-input construction, bounded seeded retries and compact temporary storage, optional cardinality metadata for imported filters, standard serialization for all three widths (Java-compatible for 8/16-bit filters), and raw parts for application-defined storage. ### Improvements diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2798be7c..b9d3393d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -170,6 +170,15 @@ If no language is specified, all languages are prepared. These directories are n The pinned TCK revision does not yet contain XOR filter snapshots. Until it does, `serde_tests/xor.rs` retains small hexadecimal fixtures generated from the merged Java implementation, with the source revision and inputs documented alongside them. Keep these separate from the synchronized `*_generated_files` directories; replace them with TCK fixtures when available. +The `java_reads_rust_images` test checks XOR interoperability in both directions using Java's actual reader and builder. It covers empty, singleton, signed-integer, and larger inputs at both supported Java fingerprint widths, including byte-preserving re-encoding of Rust images by Java. CI runs it in a dedicated required job. It requires Java 25 or later and compiled DataSketches Java classes; the ordinary Rust suite does not require a JVM. To run it locally, build Java commit `1034fb33b6e013334ff870f130b9c388cae5dfa9` with `mvn -DskipTests compile`, then run: + +```shell +DATASKETCHES_JAVA_CLASSPATH=/path/to/datasketches-java/target/classes \ + cargo test -p tests-integration --test serde_tests java_reads_rust_images -- --ignored +``` + +The test writes temporary images under the system temporary directory and removes them on success. It compares membership across independently constructed filters; their fingerprint payloads need not be identical. The 32-bit format remains covered by Rust wire-format tests until another implementation supports it. + ## Code of Conduct We expect all community members to follow our [Code of Conduct](https://www.apache.org/foundation/policies/conduct.html). diff --git a/datasketches/src/xor/filter.rs b/datasketches/src/xor/filter.rs index 0762b408..0bf297b0 100644 --- a/datasketches/src/xor/filter.rs +++ b/datasketches/src/xor/filter.rs @@ -405,6 +405,12 @@ impl XorFilter { .with_context("num_items", num_items)); } + // Assignment only needs the peel stack. Release the graph before allocating the + // fingerprint payload so these allocations do not overlap at peak memory usage. + drop(xor_mask); + drop(count); + drop(queue); + let fingerprints = match filter_type { XorFilterType::Xor8 => { let mut values = vec![0_u8; capacity].into_boxed_slice(); diff --git a/tests-integration/tests/serde_tests/XorInterop.java b/tests-integration/tests/serde_tests/XorInterop.java new file mode 100644 index 00000000..277ac132 --- /dev/null +++ b/tests-integration/tests/serde_tests/XorInterop.java @@ -0,0 +1,48 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +import java.lang.foreign.MemorySegment; +import java.nio.file.Files; +import java.nio.file.Path; +import org.apache.datasketches.filters.xorfilter.XorFilter; +import org.apache.datasketches.filters.xorfilter.XorFilterBuilder; + +class XorInterop { + public static void main(String[] args) throws Exception { + Path directory = Path.of(args[0]); + for (int bits : new int[] {8, 16}) { + for (int count : new int[] {0, 1, 4, 16384, 50001}) { + String name = bits + "_" + count + ".sk"; + byte[] image = Files.readAllBytes(directory.resolve("rust_" + name)); + XorFilter restored = XorFilter.heapify(MemorySegment.ofArray(image)); + long[] values = new long[count]; + for (int i = 0; i < count; i++) { + values[i] = i == 3 ? Long.MIN_VALUE : i + 1; + if (!restored.query(values[i])) { + throw new AssertionError("false negative: " + name + ", value " + values[i]); + } + } + if (count == 0 && restored.query(1L)) { + throw new AssertionError("empty filter matched: " + name); + } + Files.write(directory.resolve("java_" + name), + XorFilterBuilder.create(values, bits, 42).toByteArray()); + Files.write(directory.resolve("roundtrip_" + name), restored.toByteArray()); + } + } + } +} diff --git a/tests-integration/tests/serde_tests/xor.rs b/tests-integration/tests/serde_tests/xor.rs index e91fdde9..51cd2b06 100644 --- a/tests-integration/tests/serde_tests/xor.rs +++ b/tests-integration/tests/serde_tests/xor.rs @@ -339,3 +339,59 @@ fn raw_parts_preserve_an_application_storage_layout() { assert_eq!(encoded, bytes); } } + +#[test] +#[ignore = "requires Java 25+ and DataSketches Java classes; see CONTRIBUTING.md"] +fn java_reads_rust_images() { + use std::fs; + use std::process::Command; + + let classpath = std::env::var_os("DATASKETCHES_JAVA_CLASSPATH") + .expect("set DATASKETCHES_JAVA_CLASSPATH to the compiled Java classes or JAR"); + let directory = std::env::temp_dir().join(format!("xor-interop-{}", std::process::id())); + fs::create_dir(&directory).unwrap(); + for kind in [XorFilterType::Xor8, XorFilterType::Xor16] { + for count in [0, 1, 4, 16_384, 50_001_u64] { + let values = (0..count).map(|i| if i == 3 { 1 << 63 } else { i + 1 }); + let filter = XorFilterBuilder::new(kind).seed(42).build(values).unwrap(); + let name = format!("{}_{}.sk", filter.bits_per_fingerprint(), count); + fs::write( + directory.join(format!("rust_{name}")), + filter.serialize().unwrap(), + ) + .unwrap(); + } + } + let output = Command::new("java") + .arg("--class-path") + .arg(classpath) + .arg(concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/serde_tests/XorInterop.java" + )) + .arg(&directory) + .output() + .expect("run Java source launcher"); + assert!( + output.status.success(), + "Java interoperability failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + for bits in [8, 16] { + for count in [0, 1, 4, 16_384, 50_001_u64] { + let name = format!("{bits}_{count}.sk"); + assert_eq!( + fs::read(directory.join(format!("rust_{name}"))).unwrap(), + fs::read(directory.join(format!("roundtrip_{name}"))).unwrap() + ); + let bytes = fs::read(directory.join(format!("java_{name}"))).unwrap(); + let restored = XorFilter::deserialize(&bytes).unwrap(); + assert_eq!(restored.num_items(), Some(count as usize)); + for i in 0..count { + let value = if i == 3 { 1_u64 << 63 } else { i + 1 }; + assert!(restored.contains(&value), "{name}, value {value}"); + } + } + } + fs::remove_dir_all(directory).unwrap(); +} From 50871bece41dbaed5a422a4a404cf244486c21f9 Mon Sep 17 00:00:00 2001 From: tison Date: Fri, 30 Oct 2026 17:21:12 +0800 Subject: [PATCH 2/2] test(xor): defer Java interoperability to serialization snapshots --- .github/workflows/ci.yml | 27 --------- CONTRIBUTING.md | 9 --- .../tests/serde_tests/XorInterop.java | 48 ---------------- tests-integration/tests/serde_tests/xor.rs | 56 ------------------- 4 files changed, 140 deletions(-) delete mode 100644 tests-integration/tests/serde_tests/XorInterop.java diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 02842962..a83d451a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -92,31 +92,6 @@ jobs: shell: bash run: cargo x test - xor-java: - name: XOR Java interoperability - runs-on: ubuntu-24.04 - steps: - - uses: actions/checkout@v7 - - uses: actions/checkout@v7 - with: - repository: apache/datasketches-java - ref: 1034fb33b6e013334ff870f130b9c388cae5dfa9 - path: interop-java - - uses: actions/setup-java@v6 - with: - distribution: temurin - java-version: '25' - - name: Compile Java reader and builder - working-directory: interop-java - run: mvn --batch-mode -DskipTests compile - - name: Install toolchain - run: rustup toolchain install stable && rustup default stable - - uses: Swatinem/rust-cache@v2 - - name: Check both serialization directions - env: - DATASKETCHES_JAVA_CLASSPATH: ${{ github.workspace }}/interop-java/target/classes - run: cargo test -p tests-integration --test serde_tests java_reads_rust_images -- --ignored - required: name: Required runs-on: ubuntu-24.04 @@ -124,14 +99,12 @@ jobs: needs: - check - test - - xor-java steps: - name: Guardian run: | if [[ ! ( \ "${{ needs.check.result }}" == "success" \ && "${{ needs.test.result }}" == "success" \ - && "${{ needs.xor-java.result }}" == "success" \ ) ]]; then echo "Required jobs haven't been completed successfully." exit 1 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b9d3393d..2798be7c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -170,15 +170,6 @@ If no language is specified, all languages are prepared. These directories are n The pinned TCK revision does not yet contain XOR filter snapshots. Until it does, `serde_tests/xor.rs` retains small hexadecimal fixtures generated from the merged Java implementation, with the source revision and inputs documented alongside them. Keep these separate from the synchronized `*_generated_files` directories; replace them with TCK fixtures when available. -The `java_reads_rust_images` test checks XOR interoperability in both directions using Java's actual reader and builder. It covers empty, singleton, signed-integer, and larger inputs at both supported Java fingerprint widths, including byte-preserving re-encoding of Rust images by Java. CI runs it in a dedicated required job. It requires Java 25 or later and compiled DataSketches Java classes; the ordinary Rust suite does not require a JVM. To run it locally, build Java commit `1034fb33b6e013334ff870f130b9c388cae5dfa9` with `mvn -DskipTests compile`, then run: - -```shell -DATASKETCHES_JAVA_CLASSPATH=/path/to/datasketches-java/target/classes \ - cargo test -p tests-integration --test serde_tests java_reads_rust_images -- --ignored -``` - -The test writes temporary images under the system temporary directory and removes them on success. It compares membership across independently constructed filters; their fingerprint payloads need not be identical. The 32-bit format remains covered by Rust wire-format tests until another implementation supports it. - ## Code of Conduct We expect all community members to follow our [Code of Conduct](https://www.apache.org/foundation/policies/conduct.html). diff --git a/tests-integration/tests/serde_tests/XorInterop.java b/tests-integration/tests/serde_tests/XorInterop.java deleted file mode 100644 index 277ac132..00000000 --- a/tests-integration/tests/serde_tests/XorInterop.java +++ /dev/null @@ -1,48 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -import java.lang.foreign.MemorySegment; -import java.nio.file.Files; -import java.nio.file.Path; -import org.apache.datasketches.filters.xorfilter.XorFilter; -import org.apache.datasketches.filters.xorfilter.XorFilterBuilder; - -class XorInterop { - public static void main(String[] args) throws Exception { - Path directory = Path.of(args[0]); - for (int bits : new int[] {8, 16}) { - for (int count : new int[] {0, 1, 4, 16384, 50001}) { - String name = bits + "_" + count + ".sk"; - byte[] image = Files.readAllBytes(directory.resolve("rust_" + name)); - XorFilter restored = XorFilter.heapify(MemorySegment.ofArray(image)); - long[] values = new long[count]; - for (int i = 0; i < count; i++) { - values[i] = i == 3 ? Long.MIN_VALUE : i + 1; - if (!restored.query(values[i])) { - throw new AssertionError("false negative: " + name + ", value " + values[i]); - } - } - if (count == 0 && restored.query(1L)) { - throw new AssertionError("empty filter matched: " + name); - } - Files.write(directory.resolve("java_" + name), - XorFilterBuilder.create(values, bits, 42).toByteArray()); - Files.write(directory.resolve("roundtrip_" + name), restored.toByteArray()); - } - } - } -} diff --git a/tests-integration/tests/serde_tests/xor.rs b/tests-integration/tests/serde_tests/xor.rs index 51cd2b06..e91fdde9 100644 --- a/tests-integration/tests/serde_tests/xor.rs +++ b/tests-integration/tests/serde_tests/xor.rs @@ -339,59 +339,3 @@ fn raw_parts_preserve_an_application_storage_layout() { assert_eq!(encoded, bytes); } } - -#[test] -#[ignore = "requires Java 25+ and DataSketches Java classes; see CONTRIBUTING.md"] -fn java_reads_rust_images() { - use std::fs; - use std::process::Command; - - let classpath = std::env::var_os("DATASKETCHES_JAVA_CLASSPATH") - .expect("set DATASKETCHES_JAVA_CLASSPATH to the compiled Java classes or JAR"); - let directory = std::env::temp_dir().join(format!("xor-interop-{}", std::process::id())); - fs::create_dir(&directory).unwrap(); - for kind in [XorFilterType::Xor8, XorFilterType::Xor16] { - for count in [0, 1, 4, 16_384, 50_001_u64] { - let values = (0..count).map(|i| if i == 3 { 1 << 63 } else { i + 1 }); - let filter = XorFilterBuilder::new(kind).seed(42).build(values).unwrap(); - let name = format!("{}_{}.sk", filter.bits_per_fingerprint(), count); - fs::write( - directory.join(format!("rust_{name}")), - filter.serialize().unwrap(), - ) - .unwrap(); - } - } - let output = Command::new("java") - .arg("--class-path") - .arg(classpath) - .arg(concat!( - env!("CARGO_MANIFEST_DIR"), - "/tests/serde_tests/XorInterop.java" - )) - .arg(&directory) - .output() - .expect("run Java source launcher"); - assert!( - output.status.success(), - "Java interoperability failed: {}", - String::from_utf8_lossy(&output.stderr) - ); - for bits in [8, 16] { - for count in [0, 1, 4, 16_384, 50_001_u64] { - let name = format!("{bits}_{count}.sk"); - assert_eq!( - fs::read(directory.join(format!("rust_{name}"))).unwrap(), - fs::read(directory.join(format!("roundtrip_{name}"))).unwrap() - ); - let bytes = fs::read(directory.join(format!("java_{name}"))).unwrap(); - let restored = XorFilter::deserialize(&bytes).unwrap(); - assert_eq!(restored.num_items(), Some(count as usize)); - for i in 0..count { - let value = if i == 3 { 1_u64 << 63 } else { i + 1 }; - assert!(restored.contains(&value), "{name}, value {value}"); - } - } - } - fs::remove_dir_all(directory).unwrap(); -}