Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[EXP][C++] Deduplicate schemas when scanning Dataset #45340

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pitrou
Copy link
Member

@pitrou pitrou commented Jan 23, 2025

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@pitrou
Copy link
Member Author

pitrou commented Jan 23, 2025

@ursabot please benchmark

@ursabot
Copy link

ursabot commented Jan 23, 2025

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

@pitrou pitrou force-pushed the exp_deduplicate_schema branch 2 times, most recently from 9610573 to a0503f3 Compare January 23, 2025 17:23
@pitrou
Copy link
Member Author

pitrou commented Jan 23, 2025

@ursabot please benchmark

@ursabot
Copy link

ursabot commented Jan 23, 2025

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

@pitrou
Copy link
Member Author

pitrou commented Jan 23, 2025

@icexelloss This is a quick experiment that you might want to try out on a real-world use case. I don't seem to get any tangible benefits on a synthetic dataset, though it might be due to memory fragmentation.

Copy link

Thanks for your patience. Conbench analyzed the 0 benchmarking runs that have been run so far on PR commit f681035.

None of the specified runs were found on the Conbench server.

The full Conbench report has more details.

Copy link

Thanks for your patience. Conbench analyzed the 4 benchmarking runs that have been run so far on PR commit a0503f3.

There were 775 benchmark results indicating a performance regression:

The full Conbench report has more details.

@pitrou
Copy link
Member Author

pitrou commented Jan 24, 2025

Ok, there are so many unrelated "regressions" in the report above that I'm going to launch another benchmarking run, as it's likely that external factors have influenced that run.

@pitrou
Copy link
Member Author

pitrou commented Jan 24, 2025

@ursabot please benchmark

@ursabot
Copy link

ursabot commented Jan 24, 2025

Commit a0503f3 already has scheduled benchmark runs.

@pitrou pitrou force-pushed the exp_deduplicate_schema branch from a0503f3 to 335a46b Compare January 24, 2025 13:49
@pitrou
Copy link
Member Author

pitrou commented Jan 24, 2025

@ursabot please benchmark

@ursabot
Copy link

ursabot commented Jan 24, 2025

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

Copy link

Thanks for your patience. Conbench analyzed the 4 benchmarking runs that have been run so far on PR commit 335a46b.

There were 23 benchmark results indicating a performance regression:

The full Conbench report has more details.

@pitrou pitrou force-pushed the exp_deduplicate_schema branch from 335a46b to 11b7ad8 Compare January 27, 2025 14:39
@icexelloss
Copy link
Contributor

@pitrou Thanks! cc @timothydijamco

@timothydijamco
Copy link

timothydijamco commented Feb 5, 2025

Thanks for this PR, seems like a good idea to try out.

In response to #45287 (comment): I didn't observe any difference between a version of Arrow with the metadata-clearing patch (#45330) vs a version of Arrow with the metadata-clearing patch plus this patch here in this PR.

Synthetic data

500 files, each with 1 row and 10,000 columns with 200-character-long column names

Peak memory

Performing a "scan" or loading the table into memory

Only metadata-clearing (#45330) With metadata-clearing (#45330) and schema deduplication (#45340)
One "scan" (pull batches from scanner->RecordBatchReader() until exhausted) 1.49GB 1.48GB
One scanner->ToTable() 1.57GB 1.55GB

Memory profiles

Performing two "scans"

Only metadata-clearing (#45330) With metadata-clearing (#45330) and schema deduplication (#45340)
Screen Shot 2025-02-05 at 5 17 31 PM Screen Shot 2025-02-05 at 5 17 48 PM

Real data

I ran on a variety of real datasets we have internally (data size varies from <1GB of data to 40GB of data, num columns varies from hundreds to thousands, number of files varies from 1 to hundreds) in the "scan" use case and "load table" use case and did not observe any memory usage difference between the two Arrow versions as well

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

Successfully merging this pull request may close these issues.

4 participants