Add `with_skip_validation` flag to IPC `StreamReader`, `FileReader` and `FileDecoder` by alamb · Pull Request #7120 · apache/arrow-rs

alamb · 2025-02-11T22:08:06Z

Which issue does this PR close?

Supercedes Add with_skip_validation flag to IPC readers/writers #7093
Closes Improve Arrow-IPC performance by avoiding Unsafe Unchecked IPC Read RecordBatch #3287
Closes feat(arrow-ipc) Add: Support FileReader and StreamReader skip array data validation #6938

Rationale for this change

Forcing Array validation while reading arrow IPC trusted data is inefficient. Users should be able to avoid doing so if they want

What changes are included in this PR?

This PR builds on this PR from @totoroyyb

feat(arrow-ipc) Add: Support FileReader and StreamReader skip array data validation #6938

Add API in IPC options to skip validation
Pass this flag through StreamReader/FileReader/FileDecoder
Add a few more tests
Add benchmarks

Are there any user-facing changes?

New with_disable_validation APIs on StreamReader, FileReader and FileDecoder
improved performance

Benchmark results Mac M3

Observations

Enabling compression is a massive performance hit (20x slower)
Using mmap is super fast and thus the validation is a much larger portion of the time, resulting in a corresponding 8-9x faster without validation

Benchmark	Default (validation)	`with_skip_validation`	Improvement
`StreamReader`	255.59 µs	78.884 µs	3.23
`StreamReader(zstd)`	4.8023 ms	4.4646 ms	1.07
`FileReader`	251.63 µs	79.072 µs	3.18
`FileDecoder` + `mmap`	241.10 µs	26.900 µs	9.2

Details for Mac M3

   Finished `bench` profile [optimized] target(s) in 0.27s
     Running benches/ipc_reader.rs (target/release/deps/ipc_reader-a8f04a4266073085)
Gnuplot not found, using plotters backend
arrow_ipc_reader/StreamReader/read_10
                        time:   [254.38 µs 255.59 µs 256.92 µs]
                        change: [-4.5688% -3.2519% -1.9891%] (p = 0.00 < 0.05)
                        Performance has improved.
arrow_ipc_reader/StreamReader/no_validation/read_10
                        time:   [78.548 µs 78.884 µs 79.252 µs]
                        change: [-1.5389% -0.7882% -0.0139%] (p = 0.04 < 0.05)
                        Change within noise threshold.
Found 3 outliers among 100 measurements (3.00%)
  2 (2.00%) high mild
  1 (1.00%) high severe
arrow_ipc_reader/StreamReader/read_10/zstd
                        time:   [4.6568 ms 4.8023 ms 4.9459 ms]
                        change: [-13.864% -11.080% -7.8912%] (p = 0.00 < 0.05)
                        Performance has improved.
arrow_ipc_reader/StreamReader/no_validation/read_10/zstd
                        time:   [4.3195 ms 4.4646 ms 4.6121 ms]
                        change: [-17.118% -14.219% -11.026%] (p = 0.00 < 0.05)
                        Performance has improved.
arrow_ipc_reader/FileReader/read_10
                        time:   [251.14 µs 251.63 µs 252.16 µs]
                        change: [-3.3146% -2.7427% -2.2098%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 8 outliers among 100 measurements (8.00%)
  8 (8.00%) high mild
arrow_ipc_reader/FileReader/no_validation/read_10
                        time:   [78.319 µs 79.072 µs 79.939 µs]
                        change: [-14.299% -13.005% -11.710%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 10 outliers among 100 measurements (10.00%)
  6 (6.00%) high mild
  4 (4.00%) high severe
arrow_ipc_reader/FileReader/read_10/mmap
                        time:   [240.51 µs 241.10 µs 241.78 µs]
                        change: [-1.0442% -0.6140% -0.2258%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 8 outliers among 100 measurements (8.00%)
  6 (6.00%) high mild
  2 (2.00%) high severe
arrow_ipc_reader/FileReader/no_validation/read_10/mmap
                        time:   [26.816 µs 26.900 µs 26.994 µs]
                        change: [-1.3681% -0.7452% -0.1508%] (p = 0.02 < 0.05)
                        Change within noise threshold.
Found 10 outliers among 100 measurements (10.00%)
  5 (5.00%) low mild
  3 (3.00%) high mild
  2 (2.00%) high severe

Benchmarks: GCP

On a GCP c2-standard-16 (16 vCPUs, 64 GB Memory)

Benchmark	Default (validation)	`with_skip_validation`	Improvement
`StreamReader`	665.40 µs	419.04 µs	1.58
`StreamReader(zstd)`	3.4684 ms	3.2056 ms	1.08
`FileReader`	664.86 µs	406.10 µs	1.63
`FileDecoder` + `mmap`	530.24 µs	77.129 µs	6.9

Raw Results (gpc)

arrow_ipc_reader/StreamReader/read_10
                        time:   [664.17 µs 665.40 µs 666.63 µs]
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild
arrow_ipc_reader/StreamReader/no_validation/read_10
                        time:   [417.36 µs 419.04 µs 420.93 µs]
Found 5 outliers among 100 measurements (5.00%)
  5 (5.00%) high mild
arrow_ipc_reader/StreamReader/read_10/zstd
                        time:   [3.4657 ms 3.4684 ms 3.4711 ms]
Found 3 outliers among 100 measurements (3.00%)
  2 (2.00%) high mild
  1 (1.00%) high severe
arrow_ipc_reader/StreamReader/no_validation/read_10/zstd
                        time:   [3.2021 ms 3.2056 ms 3.2095 ms]
Found 7 outliers among 100 measurements (7.00%)
  2 (2.00%) high mild
  5 (5.00%) high severe
arrow_ipc_reader/FileReader/read_10
                        time:   [663.60 µs 664.86 µs 666.13 µs]
Found 10 outliers among 100 measurements (10.00%)
  3 (3.00%) low mild
  6 (6.00%) high mild
  1 (1.00%) high severe
arrow_ipc_reader/FileReader/no_validation/read_10
                        time:   [400.17 µs 406.10 µs 411.81 µs]
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) low mild
arrow_ipc_reader/FileReader/read_10/mmap
                        time:   [529.08 µs 530.24 µs 531.62 µs]
Found 14 outliers among 100 measurements (14.00%)
  7 (7.00%) high mild
  7 (7.00%) high severe
arrow_ipc_reader/FileReader/no_validation/read_10/mmap
                        time:   [77.066 µs 77.129 µs 77.203 µs]
Found 7 outliers among 100 measurements (7.00%)
  1 (1.00%) low mild
  4 (4.00%) high mild
  2 (2.00%) high severe

…nd `FileDecoder`

alamb · 2025-02-12T11:48:09Z

arrow-data/src/data.rs


-mod private {
-    /// A boolean flag that cannot be mutated outside of unsafe code.
+/// A boolean flag that cannot be mutated outside of unsafe code.


I propose to make this UnsafeFlag public (and added examples and more docs) so I could use it across the two crates. However, I can also make a private copy of it in arrow-ipc if reviewers feel it would be better to avoid a new API

alamb · 2025-02-12T11:50:42Z

arrow-ipc/benches/ipc_reader.rs

-            writer.write(&batch).unwrap();
-        }
-        writer.finish().unwrap();
+        let buffer = ipc_stream();


I added new versions of each benchmark that work with disabled validation

alamb · 2025-02-12T11:51:34Z

arrow-ipc/src/reader.rs

-                    StructArray::try_new(struct_fields.clone(), struct_arrays, None)?
-                };
-                Ok(Arc::new(struct_array))
+                self.create_struct_array(struct_node, null_buffer, struct_fields, struct_arrays)


I refactored this code into its own function so it was eaiser to call StructArray::new_unchecked when validation was disabled

alamb · 2025-02-12T11:52:38Z

arrow-ipc/src/reader.rs

+    ///
+    /// Relies on the caller only passing a flag with `true` value if they are
+    /// certain that the data is valid
+    pub fn with_skip_validation(mut self, skip_validation: UnsafeFlag) -> Self {


Note that RecordBatchDecoder is not a public API:
https://docs.rs/arrow-ipc/54.1.0/arrow_ipc/reader/struct.StreamReader.html?search=RecordBatchDecoder

alamb · 2025-02-12T11:53:01Z

arrow-ipc/src/reader.rs

        self
    }

+    /// Specifies if validation should be skipped when reading data (defaults to `false`)


this is a new public API and follows the same pattern as ArrayData::skip_validation

alamb · 2025-02-12T11:53:10Z

arrow-ipc/src/reader.rs

        &mut self.reader
    }
+
+    /// Specifies if validation should be skipped when reading data (defaults to `false`)


alamb · 2025-02-12T11:53:16Z

arrow-ipc/src/reader.rs

        &mut self.reader
    }
+
+    /// Specifies if validation should be skipped when reading data (defaults to `false`)


alamb · 2025-02-12T11:54:10Z

arrow-ipc/src/reader.rs

    }

+    #[test]
+    fn test_invalid_nested_array_ipc_read_errors() {


I added some additional coverage to make sure the flag got passed when decoding multiple nesting levels

alamb · 2025-02-12T11:54:28Z

arrow-ipc/src/reader.rs

    }

+    #[test]
+    fn test_validation_of_invalid_union_array() {


Turns out UnionArray had its own path / handling so new coverage added

alamb · 2025-02-12T11:55:01Z

arrow-ipc/src/reader.rs


        // IPC Stream format
        let buf = write_stream(&rb); // write is ok
+        read_stream_skip_validation(&buf).unwrap();


Now all the validation tests also verify they can read the batch back without error if validation is disabled

tustvold

Mostly just some minor nits

tustvold · 2025-02-24T17:45:39Z

arrow-ipc/benches/ipc_reader.rs

+}
+
+/// Return an IPC stream with ZSTD compression with 10 record batches
+fn ipc_stream_zstd() -> Vec<u8> {


Minor nit, but I wonder if we could have an ipc_stream_options that takes a IpcWriteOptions and then have ipc_stream just call through to this

this was a good suggestion -- I did it in 7d3f020 🧹

arrow-ipc/src/reader.rs

arrow-data/src/data.rs

…idation

Co-authored-by: Raphael Taylor-Davies <[email protected]>

…-rs into alamb/ipc_disable_validation

Co-authored-by: Raphael Taylor-Davies <[email protected]>

…-rs into alamb/ipc_disable_validation

alamb · 2025-02-26T17:11:07Z

Thank you very much for the review @tustvold

github-actions bot added the arrow Changes to the arrow crate label Feb 11, 2025

alamb mentioned this pull request Feb 11, 2025

Add with_skip_validation flag to IPC readers/writers #7093

Closed

3 tasks

alamb force-pushed the alamb/ipc_disable_validation branch from d4102d6 to 59b3033 Compare February 11, 2025 22:12

alamb added 2 commits February 12, 2025 06:45

Make UnsafeFlag pub

0c206c6

Add with_skip_validation flag to IPC StreamReader, FileReader a…

77d3de5

…nd `FileDecoder`

alamb force-pushed the alamb/ipc_disable_validation branch from 7b85e5e to 77d3de5 Compare February 12, 2025 11:45

alamb commented Feb 12, 2025

View reviewed changes

alamb marked this pull request as ready for review February 12, 2025 14:58

tustvold approved these changes Feb 24, 2025

View reviewed changes

alamb and others added 8 commits February 26, 2025 11:55

Merge remote-tracking branch 'apache/main' into alamb/ipc_disable_val…

75f9ca6

…idation

Apply suggestions from code review

b0620af

Co-authored-by: Raphael Taylor-Davies <[email protected]>

Merge branch 'alamb/ipc_disable_validation' of github.com:alamb/arrow…

63e1d1a

…-rs into alamb/ipc_disable_validation

Update arrow-ipc/src/reader.rs

932d06a

Co-authored-by: Raphael Taylor-Davies <[email protected]>

Update arrow-ipc/src/reader.rs

8ae8aa2

Co-authored-by: Raphael Taylor-Davies <[email protected]>

Merge branch 'alamb/ipc_disable_validation' of github.com:alamb/arrow…

1adffb5

…-rs into alamb/ipc_disable_validation

Add notes to RecordBatchDecoder::with_skip_validation flag

1043301

remove repetition in read benchmark

7d3f020

alamb merged commit 2fd5181 into apache:main Feb 27, 2025
26 checks passed

alamb deleted the alamb/ipc_disable_validation branch February 27, 2025 12:24

milenkovicm mentioned this pull request Feb 27, 2025

Consider using with_skip_validation for shuffle file reading apache/datafusion-ballista#1189

Closed

alamb mentioned this pull request Feb 28, 2025

Optimized spill file format apache/datafusion#14078

Closed

3 tasks

alamb added performance enhancement Any new improvement worthy of a entry in the changelog labels Mar 15, 2025

This was referenced Mar 19, 2025

Improve spill performance: Disable re-validation of spilled files apache/datafusion#15320

Closed

Improve Spill Performance: mmap the spill files apache/datafusion#15321

Open

Comments

Conversation

alamb commented Feb 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

Benchmark results Mac M3

Benchmarks: GCP

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tustvold left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alamb commented Feb 26, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

alamb commented Feb 11, 2025 •

edited

Loading