Skip to content

Conversation

@HyukjinKwon
Copy link
Member

@HyukjinKwon HyukjinKwon commented Jan 20, 2026

Rationale for this change

StructArray diffing was inefficient because it compared all fields together with no early termination. For a 10-field struct, even if the first field differs, it checks all 10 fields.

What changes are included in this PR?

Introduced StructValueComparator that compares struct fields independently with early termination.

Are these changes tested?

Yes, added DiffTest.StructFieldComparison and DiffTest.NestedStructComparison
All existing struct diff tests pass. Plus, I verified this with local benchmark (can be shared if asked).

Are there any user-facing changes?

No behavioral changes but optimization for Array.diff().

@github-actions
Copy link

⚠️ GitHub issue #48906 has been automatically assigned in GitHub to PR creator.

@HyukjinKwon HyukjinKwon force-pushed the optimize-struct-diff branch 2 times, most recently from 54c6b2b to 487eaf2 Compare January 21, 2026 03:42
@rok
Copy link
Member

rok commented Jan 21, 2026

@ursabot please benchmark

@rok
Copy link
Member

rok commented Jan 21, 2026

Benchmark runs are scheduled for commit 2a8a9ab. Watch https://buildkite.com/apache-arrow and https://conbench.arrow-dev.org for updates. A comment will be posted here when the runs are complete.

@rok
Copy link
Member

rok commented Jan 21, 2026

@HyukjinKwon sorry for the noise

@HyukjinKwon
Copy link
Member Author

No please let me know if there is anything to do to verify the fix 🙇‍♂️

@rok
Copy link
Member

rok commented Jan 21, 2026

No please let me know if there is anything to do to verify the fix 🙇‍♂️

I was just testing our benchmarking infra, it's unrelated to your change not much you can do directly on github :).

@conbench-apache-arrow
Copy link

Thanks for your patience. Conbench analyzed the 3 benchmarking runs that have been run so far on PR commit 2a8a9ab.

There was 1 benchmark result with an error:

There were 2 benchmark results indicating a performance regression:

The full Conbench report has more details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants