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

Improve parquet ListingTable speed with parquet metadata (short clickbench queries) #11719

Open
alamb opened this issue Jul 30, 2024 · 6 comments
Assignees
Labels
enhancement New feature or request

Comments

@alamb
Copy link
Contributor

alamb commented Jul 30, 2024

Is your feature request related to a problem or challenge?

I spent some time looking at the ClickBench results with DataFusion 40.0.0
#11567 (comment) (thanks @pmcgleenon 🙏 )

Specifically, I looked into how we could make some of the already fast queries on the the partitioned dataset faster. Unsurprisingly, for the really fast queries the query time is actually dominated by parquet metadata analysis and DataFusion statistics creation.

For example

ClickBench Q0

SELECT COUNT(*) FROM hits;

To reproduce, run:

cd datafusion
cargo run --release --bin dfbench -- clickbench --iterations 100 --path benchmarks/data/hits_partitioned  --query 0

I profiled this using Instruments. Here are some annotated screenshots

Screenshot 2024-07-30 at 6 25 43 AM Screenshot 2024-07-30 at 6 26 53 AM

Some of my take aways are

  1. a substantial amount of time is spent reading the parquet metadata twice
  2. A substantial amount of time is spent managing the ScalarValues in statistics

Describe the solution you'd like

If would be cool to make these queries faster by reducing the per file metadata handling overhead (e.g. don't read the metadata more than once and figure out some way to make statistics handling more efficient)

Describe alternatives you've considered

Note this project isn't broken down into tasks yet

I think @Ted-Jiang did some work way back to cache parquet metaddata

Additional context

No response

@Rachelint
Copy link
Contributor

Rachelint commented Jul 30, 2024

Some ideas about solving

A substantial amount of time is spent managing the ScalarValues in statistics

Plan to try it today.

See one simple thing is, refactor the Statistics to:

pub struct StatisticsInner {
    /// The number of table rows.
    pub num_rows: Precision<usize>,
    /// Total bytes of the table rows.
    pub total_byte_size: Precision<usize>,
    /// Statistics on a column level. It contains a [`ColumnStatistics`] for
    /// each field in the schema of the table to which the [`Statistics`] refer.
    pub column_statistics: Vec<ColumnStatistics>,
}

pub struct Statistics {
   inner: Arc<StatisticsInner>,
}

And the clone of Arc is trivial.

@Rachelint
Copy link
Contributor

take

@alamb
Copy link
Contributor Author

alamb commented Jul 30, 2024

That would be a very interesting experiment to try

@Rachelint
Copy link
Contributor

Rachelint commented Aug 3, 2024

Based on the detail profile about q0 in clickbench as following, maybe the optimization work can be divided into three parts:

  • Reduce the cost about clone and drop of Statistics
  • Maybe optimize the impl for get_statistics_with_limit(seems may tmp vectors exist, but not sure)
  • Do cache for the result of object store list operation

Trying the first possible optimization now.

dperf

@alamb
Copy link
Contributor Author

alamb commented Aug 5, 2024

#11802 is very nice 👌 It would be fascinating to know what the flamegraph looks like after that PR (aka what are next highest bottleneck)

@Rachelint
Copy link
Contributor

#11802 is very nice 👌 It would be fascinating to know what the flamegraph looks like after that PR (aka what are next highest bottleneck)

😄 I guess they will be the plan creation and object store list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants