Reduce overhead of parquet metric creation

@alamb

I wonder if we should apply the same pattern to the other metrics (lazily initialize them) -- if you can get a few percent in this query maybe it would get us a few in the others

Originally posted by @alamb in #21637 (comment)

I think this will especially help the shorter clickbench queries where the overhead of opening the parquet files is substantial

@xudong963

Update here

@xudong963 tried this in Lazy register parquet file metrics #22353 🏆

However, upon review I think it turns out we can't use lazy registration as that would change the semantics

Lazy register parquet file metrics #22353 (review)

However, I do think we could reduce the overhead of creating metrics in general by avoiding the string allocations. I am copying my comments from #22353 (comment) here as I think they are relevant:

One challenge is that there are many many new strings with the file name. Maybe it is worth looking into if we could replace the owned string in Label with a Arc (as another PR)

datafusion/datafusion/physical-expr-common/src/metrics/mod.rs

Lines 526 to 527 in 3aefba7

name: Cow<'static, str>,

value: Cow<'static, str>,

It might be tricky however, as we would have to retain the special case for &'static str 🤔

Maybe we could make a new thing instead of Cow like

enum ArcCow {
  /// Static string
  Static(&'static str),
  /// Shared arc
  Shared(Arc<str>),
   /// Owned allocation
   Owned(String)
}

With some finagling (like impl Into<ArcCow> I suspect we could make it mostly backwards compatible with the existing APIs / callsites 🤔

Maybe

enum SharedStr {
  /// Static string
  Static(&'static str),
  /// Shared arc
  Shared(Arc<str>),
   /// Owned allocation
   Owned(String)
}

And instead of

        let row_groups_pruned_bloom_filter = MetricBuilder::new(metrics)
            .with_new_label("filename", filename.to_string())
            .with_type(MetricType::Summary)
            .pruning_metrics("row_groups_pruned_bloom_filter", partition);

        let limit_pruned_row_groups = MetricBuilder::new(metrics)
            .with_new_label("filename", filename.to_string())
            .with_type(MetricType::Summary)
            .pruning_metrics("limit_pruned_row_groups", partition);

It could look something like

        // Allocate a single copy of the filename
        let shared_filename = Arc::from(filename.as_str());

        let row_groups_pruned_bloom_filter = MetricBuilder::new(metrics)
            .with_new_label("filename", Arc::clone(&shared_filename))     // <---- Only copy Arc here, not new string
            .with_type(MetricType::Summary)
            .pruning_metrics("row_groups_pruned_bloom_filter", partition);  

        let limit_pruned_row_groups = MetricBuilder::new(metrics)
            .with_new_label("filename", Arc::clone(&shared_filename))     // <---- Only copy Arc here, not new string
            .with_type(MetricType::Summary)
            .pruning_metrics("limit_pruned_row_groups", partition);

Optimize metric label cloning (apache#22406)

We hit this one as a memory problem rather than a latency one, so here are some numbers in case they help.

Setup: DataFusion 54.1.0, via Sail 0.7.1.

Query: select x, count(*) … group by x over a hive-partitioned parquet table.
Table: a wide schema (488 columns, the union of many sources). Most files are small, about 1,100 files per day of data.
Server: one process with a 3.9 GB cgroup limit, 4 partitions, statistics collection off.

What we saw. Peak memory grew linearly with the number of files scanned, about 32 MB per day (about 29 KB per file). A 27-day scan peaked at about 915 MB. A jemalloc heap profile, unsampled, put about 85% of the peak under ParquetFileMetrics::new. The plan's ExecutionPlanMetricsSet keeps every registered metric until the query ends, so these sets add up over the whole scan rather than being freed file by file.

Two things we noticed in 54.1.0:

Duplicate registration: each opened file gets two ParquetFileMetrics, one from the reader factory (DefaultParquetFileReaderFactory::create_reader) and one from ParquetMorselizer::prepare_open_file, plus a BaselineMetrics per file. That is about 40 filename-labelled metrics per file. Sharing a single instance per file would halve this without changing what users see, unless I'm missing a reason they're separate.
Optimize metric label cloning #22406 doesn't change the count: sharing the label string removes the copies, but the number of Metric entries still grows with the number of files.

What we tried (local patch, not proposed as-is). Each scan partition registers one unlabelled ParquetFileMetrics and one BaselineMetrics on first use. Every file opened on that partition adds to the same counters. The reader keeps its own per-file byte count, so the scan-efficiency ratio stays correct.

Results, same query and data, run alternately:

Days scanned	54.1.0 peak	patched peak	time
14	505 MB	188 MB	103.5 s → 101.8 s
27	913–921 MB	344–350 MB	205–231 s → 179–197 s (13–15% faster)

Results were identical. Metrics are still registered eagerly and visible during execution, which I understand was the concern with #22353, and EXPLAIN ANALYZE totals don't change. The trade-off is that per-file metric rows and the filename label disappear, so this changes observable output. That's fine for us, because we only look at aggregates, but it probably shouldn't be the default.

A possible shape upstream: a parquet option, say per_file_metrics (default true), that falls back to per-partition metrics on scans of very many files. That, together with (1), would cover the case of wide tables with many small files.

Happy to share the patch or turn this into a PR if one of these directions sounds acceptable.

What we saw. Peak memory grew linearly with the number of files scanned, about 32 MB per day (about 29 KB per file). A 27-day scan peaked at about 915 MB. A jemalloc heap profile, unsampled, put about 85% of the peak under ParquetFileMetrics::new. The plan's ExecutionPlanMetricsSet keeps every registered metric until the query ends, so these sets add up over the whole scan rather than being freed file by file.

That sounds embarassing. Maybe we should have some limit or something that caps the number of metrics (as keeping per file statsistics for 14 x 1,000 files is expensive and will likely be of limited use

A possible shape upstream: a parquet option, say per_file_metrics (default true), that falls back to per-partition metrics on scans of very many files. That, together with (1), would cover the case of wide tables with many small files.

yes I think this makes a lot of sense

Duplicate registration: each opened file gets two ParquetFileMetrics, one from the reader factory (DefaultParquetFileReaderFactory::create_reader) and one from ParquetMorselizer::prepare_open_file, plus a BaselineMetrics per file. That is about 40 filename-labelled metrics per file. Sharing a single instance per file would halve this without changing what users see, unless I'm missing a reason they're separate.

reducing duplication also make s sense to me -- please create a PR for each of these ideas and tag me on it and I will be happy to review

@xudong963

17. reducing duplication also make s sense to me -- please create a PR for each of these ideas and tag me on it and I will be happy to review

+1, feel free to @xudong963

fix(parquet): register one set of ParquetFileMetrics per opened file

Navigation Menu

Repository navigation

Reduce overhead of parquet metric creation #22189

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Reduce overhead of parquet metric creation #22189

Description

Activity

alamb commented on May 15, 2026

alamb commented on May 20, 2026

alamb commented on May 20, 2026

alamb commented on May 20, 2026

vaibhavbodhane commented on Oct 29, 2026

alamb commented on Oct 29, 2026

alamb commented on Oct 29, 2026

alamb commented on Oct 29, 2026

xudong963 commented on Oct 30, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions