Skip to content

perf(catalog-listing): cut datafusion-catalog-listing compile time ~4.4x - #24330

Merged
Dandandan merged 1 commit into
apache:mainfrom
Dandandan:perf/catalog-listing-compile-time
Aug 13, 2026
Merged

perf(catalog-listing): cut datafusion-catalog-listing compile time ~4.4x#24330
Dandandan merged 1 commit into
apache:mainfrom
Dandandan:perf/catalog-listing-compile-time

Conversation

@Dandandan

@Dandandan Dandandan commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Adresses: #13814

Fourth and last crate in the family from #24325 (datafusion-catalog), #24326
(datafusion-session) and #24329 (datafusion core). Independent of all three —
different crates, so they can merge in any order.

Rationale for this change

datafusion-catalog-listing is 3,013 lines of source but spends 20.5s in the
frontend during a cold cargo build -p datafusion (cargo build --timings), and
it sits on the critical path between datafusion-catalog and datafusion core.

-Zself-profile puts 58% of the crate's compile time in evaluate_obligation,
and grouping those goals by the Self type in their ParamEnv shows all of it
in one impl:

Self in ParamEnv time goals
ListingTable 3.16s 6,967
(empty ParamEnv) 0.04s 8,443

Note the second row — the same kind of goals cost ~5µs each with an empty
ParamEnv against ~450µs here.

The cause is the one from the earlier PRs: #[async_trait] gives each async fn
a where 'life0: 'async_trait, .. clause, which makes the method's ParamEnv
non-empty, and rustc only serves auto-trait obligations from its global
evaluation cache when the ParamEnv is empty. So the Send/Sync proof for
everything the future captures is redone per method.

All three async methods in this impl reach Expr:

  • scan takes &[Expr]
  • scan_with_args takes ScanArgs<'a>, which holds &[Expr]
  • insert_into keeps self.options (Vec<Vec<SortExpr>>) live across an await

so each one pays for a walk of the whole Expr/LogicalPlan graph.

What changes are included in this PR?

Each method is now the hand-written desugaring of async fn and only forwards;
the coroutine is built in a shim with no where-clauses, so its proofs land in the
global cache. Bodies are moved verbatim into inherent fns and all three stay
async, so nothing is evaluated any earlier than before —
Box::pin(self.m_inner(..)) polls nothing.

Following the review on #24326, I measured each method's marginal contribution
first, by reverting one at a time (together with its helpers) from the
all-converted state:

state crate build evaluate_obligation
all three converted 0.931s 34.9ms
revert insert_into 1.866s 1.01s
revert scan 1.943s 1.05s
revert scan_with_args 2.118s 1.17s
none converted (base) 4.201s 3.25s

Unlike #24326 — where two of the seven bodies I first converted turned out to
gain nothing — all three pull their weight here. Converting all of them leaves no
coroutine in the impl at all, so the graph is never walked in a non-empty
ParamEnv, which is why the total drops by two orders of magnitude rather than
by a third.

Are these changes tested?

  • cargo test -p datafusion-catalog-listing — 18 + 7 passed
  • cargo test -p datafusion --lib — 442 passed
  • cargo check -p datafusion --all-targets — clean (ListingTable is used
    heavily by core's integration tests and benches)
  • cargo clippy -p datafusion-catalog-listing --all-targets — clean
  • cargo fmt --check — clean

The compiler checks each rewritten signature against the trait declaration, and
every body is moved verbatim.

Interleaved A/B of cargo rustc -p datafusion-catalog-listing --lib, alternating
3 times so machine drift cancels out:

base: 4.257s  4.189s  4.134s
fix:  0.984s  0.935s  0.980s

evaluate_obligation drops from 3.25s to 34.7ms.

Are there any user-facing changes?

No. No public signature changes — after macro expansion these methods have the
same signatures as before.

Follow-up

With this, the four crates that made up the serial tail of a cold build are done.
The general fix remains available and would cover downstream implementors too:
drop #[async_trait] from these traits in favour of an explicit BoxFuture
return with a single lifetime and no where-clauses, so that every impl is cheap
without hand-desugaring. That is a breaking change to public traits, so it is out
of scope here.

🤖 Generated with Claude Code

@github-actions github-actions Bot added the catalog Related to the catalog crate label Aug 13, 2026
@codecov-commenter

codecov-commenter commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 81.14%. Comparing base (ab12f5e) to head (bac3145).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
datafusion/catalog-listing/src/table.rs 99.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main   #24330    +/-   ##
========================================
  Coverage   81.14%   81.14%            
========================================
  Files        1112     1112            
  Lines      386933   387150   +217     
  Branches   386933   387150   +217     
========================================
+ Hits       313967   314158   +191     
- Misses      54476    54485     +9     
- Partials    18490    18507    +17     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…~4.4x

Fourth and last crate in the family from apache#24325, apache#24326 and apache#24329. 58% of this
crate's compile time was the trait solver (`evaluate_obligation`), and all 3.16s
of it came from the single `impl TableProvider for ListingTable`.

`#[async_trait]` gives each `async fn` a `where 'life0: 'async_trait, ..`
clause. rustc only serves auto-trait obligations from its global evaluation
cache when the `ParamEnv` is empty, so the `Send`/`Sync` proof for everything
the returned future captures is redone per method. All three async methods here
reach `Expr` -- `scan` takes `&[Expr]`, `scan_with_args` takes `ScanArgs<'a>`
which holds `&[Expr]`, and `insert_into`'s body keeps `self.options`
(`Vec<Vec<SortExpr>>`) live across an await -- so each pays for a walk of the
whole `Expr`/`LogicalPlan` graph.

Each method is now the hand-written desugaring of `async fn` and only forwards;
the coroutine is built in a shim with no where-clauses, so its proofs land in
the global cache. Bodies are moved verbatim into inherent fns, all three still
`async`, so nothing is evaluated any earlier than before.

Measured per method, by reverting one at a time (with its helpers) from the
all-converted state:

    all three converted     0.931s   obligations 34.9ms
    revert insert_into      1.866s   obligations 1.01s
    revert scan             1.943s   obligations 1.05s
    revert scan_with_args   2.118s   obligations 1.17s
    none converted (base)   4.201s   obligations 3.25s

Unlike apache#24326, all three pull their weight: converting all of them leaves no
coroutine in the impl at all, so the graph is never walked in a non-empty
`ParamEnv`.

Interleaved A/B of `cargo rustc -p datafusion-catalog-listing --lib`, 3 pairs:

    base: 4.257s  4.189s  4.134s
    fix:  0.984s  0.935s  0.980s

`evaluate_obligation` drops 3.25s -> 34.7ms.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Dandandan
Dandandan force-pushed the perf/catalog-listing-compile-time branch from 8431010 to bac3145 Compare August 13, 2026 16:59
@Dandandan
Dandandan added this pull request to the merge queue Aug 13, 2026
Merged via the queue into apache:main with commit 519687c Aug 13, 2026
38 checks passed
@Dandandan
Dandandan deleted the perf/catalog-listing-compile-time branch August 13, 2026 17:36
ryux1 pushed a commit to ryux1/datafusion that referenced this pull request Aug 13, 2026
## Which issue does this PR close?

Adresses: apache#13814

First of four; see also apache#24326 (`datafusion-session`), apache#24329
(`datafusion` core)
and apache#24330 (`datafusion-catalog-listing`). All independent — different
crates, so
they can merge in any order.

## Rationale for this change

`datafusion-catalog` is only 4.9k lines of source, but it takes **42s**
of a cold
`cargo build -p datafusion` (measured with `cargo build --timings`).

`-Zself-profile` says ~90% of the crate's compile time is
`evaluate_obligation`,
and ~99% of that is proving `Send`/`Sync`:

| trait | time | goals |
|---|---|---|
| `Send` | 4.04s | 8245 |
| `Sync` | 3.97s | 8195 |
| everything else | 0.03s | 7355 |
51.8s of trait
solving:

| impl | trait solving |
|---|---|
| `MemTable` | 13.7s |
| `StreamTable` | 9.0s |
| `CteWorkTable` | 8.9s |
| `StreamWrite` | 6.9s |
| `StreamTableFactory` | 4.5s |
| `ViewTable` | 4.4s |
| `StreamingTable` | 4.4s |

## What changes are included in this PR?

For those impls, the future is now constructed in a small shim function
that has
**no** where-clauses, so its auto-trait obligations are proved in an
empty
`ParamEnv` and get cached globally. The trait method is left as a
hand-written
desugaring of what `#[async_trait]` would have generated, and only
forwards — it
never creates a coroutine of its own, so it does no auto-trait work.



Isolated probe confirming the shape is what matters (5 trivial impls of
a local
`#[async_trait]` trait taking `&[Expr]`, added to this crate):

| variant | crate build | cost of the 5 impls |
|---|---|---|
| no impls (baseline) | 8.31s | — |
| `#[async_trait]` + `async fn` | 11.64s | +3.33s |
| `async fn` delegating body to a boxed helper | 14.28s | +5.97s |
| desugared signature + boxed shim | 8.01s | ~0 |

Note the middle row: moving only the *body* out makes things worse. The
`async fn`
itself has to go, because its arguments are what the future captures.

## Are these changes tested?


The change is mechanical and the compiler checks each rewritten
signature against
the trait declaration.

Interleaved A/B of `cargo rustc -p datafusion-catalog --lib`,
alternating 3 times
so machine drift cancels out:

```
before: 8.08s  7.90s  7.63s
after:  1.77s  1.70s  1.69s
```

`evaluate_obligation` drops from **7.31s to 70ms**, and its goal count
from
25,811 to 14,934. In a full `cargo build -p datafusion` the crate's unit
goes from
42.3s to ~8s; since it sits alone on the critical path, that time comes
straight
off the build's wall clock.

## Are there any user-facing changes?

No. No public signature changes — after macro expansion the trait
methods have the
same signatures as before.



🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
ryux1 pushed a commit to ryux1/datafusion that referenced this pull request Aug 13, 2026
## Which issue does this PR close?

Adresses: apache#13814

Third and largest instance of the problem from apache#24325
(`datafusion-catalog`),
apache#24326 (`datafusion-session`) and apache#24330 (`datafusion-catalog-listing`).
Independent of all of them — different crates, so they can merge in any
order.

## Rationale for this change

`datafusion` core is the last unit of a cold `cargo build -p datafusion`
and
compiles alone, so its cost lands directly on the build's wall clock.
**76% of
its compile time was the trait solver**: `-Zself-profile` reported 75.0s
of
`evaluate_obligation` out of 98.6s total, essentially all of it proving
`Send`/`Sync`.

`#[async_trait]` gives each `async fn` a `where 'life0: 'async_trait,
..`
clause. rustc only serves auto-trait obligations from its **global**
evaluation
cache when the `ParamEnv` is empty, so the `Send`/`Sync` proof for
everything
the returned future captures is redone per method. In this crate the
captured
sets include `SessionState`, `&LogicalPlan` and `ListingTableConfig`,
each of
which reaches a large fraction of the logical-plan type graph.

Grouping the goals by the `Self` type in their `ParamEnv` shows how
concentrated
this was — 14 impls, top 10 = 78% of the total:

| impl | trait solving | | impl | trait solving |
|---|---|---|---|---|
| `ParquetReadOptions` | 7.73s | | `DynamicListTableFactory` | 5.18s |
| `JsonReadOptions` | 7.54s | | `ListingTableFactory` | 4.56s |
| `DefaultPhysicalPlanner` | 7.07s | | `TestTableFactory` | 4.47s |
| `CsvReadOptions` | 6.59s | | `ListingTableConfig` | 4.30s |
| `DataFrameTableProvider` | 5.68s | | `DefaultQueryPlanner` | 4.28s |
| *(trait default bodies)* | 5.37s | | `DefaultTableFactory` | 4.15s |
| | | | `SessionState` | 4.11s |
| | | | `ArrowReadOptions` | 3.72s |

For contrast, in the same compile 31,818 goals with an **empty**
`ParamEnv` cost
0.19s in total — 6µs each, against ~1.3ms for the same kind of goal
under
`async_trait`'s bounds.

## What changes are included in this PR?

**First commit.** Each of those methods becomes the hand-written
desugaring of
`async fn`, which only forwards; the coroutine is built in a shim with
no
where-clauses, so its auto-trait obligations are proved in an empty
`ParamEnv`
and land in the global cache. Method bodies are moved verbatim into
inherent fns.

The `ReadOptions` family (25.6s across four impls, plus the 5.37s
default body)
collapses to a **single** proof: all five impls already delegated to the
`_get_resolved_schema` default body, which now hands the coroutine to a
free
`infer_schema_boxed`. Because that helper is a plain function with no
generics
and no where-clauses, its proof is cached once and shared by every impl.

**Second commit**, from re-profiling after the first. 11.15s of trait
solving
remained, in exactly two places:

1. `ListingTableConfigExt::infer` was still an `async fn` capturing
   `self: ListingTableConfig` (4.37s). It now uses the same shim as
   `infer_options` beside it.
2. `ReadOptions::_get_resolved_schema` still carried `Self: Sync`, which
`#[async_trait]` needed while its body was a coroutine capturing
`&self`.
After the first commit it is neither, so the bound is dead weight — and
it
forced every caller to prove its own type `Sync` structurally, through
arrow's `DataType`/`Schema`, in a non-empty `ParamEnv` (2.1–2.4s each
for
Csv/Json/Parquet; `ArrowReadOptions` was already cheap, having fewer
fields).

Two things worth noting for review:

- `DefaultPhysicalPlanner::create_initial_plan` already used exactly
this shape
(`-> BoxFuture<'a, _>` plus `Box::pin(async move ..)`) — there for
recursion
  rather than for compile time. The idiom is not new to this codebase.
- The second commit **relaxes a bound on a public trait method**.
Nothing in tree
  overrides `_get_resolved_schema` (all five impls only implement
`get_resolved_schema`) and the underscore prefix marks it as internal,
but an
external override written with `#[async_trait]` would generate `Self:
Sync` and
no longer match. Happy to drop that commit if you would rather not touch
it.

One body became eager: `TestTableFactory::create_inner` has no `.await`,
so it is
a plain fn wrapped in `ready(..)`. It builds a `TestTableProvider` and
has no side
effects. Everything that awaits stays lazy —
`Box::pin(self.m_inner(..))` polls
nothing.

## Are these changes tested?

- `cargo test -p datafusion --lib` — 442 passed
- `cargo check -p datafusion --all-targets` — clean (covers core's
integration
  tests and benches, heavy users of these APIs)
- `cargo clippy -p datafusion --lib` — clean
- `cargo doc` with `-D warnings` — clean
- `cargo fmt --check` — clean

The compiler checks each rewritten signature against its trait
declaration, and
every body is moved verbatim.

`cargo rustc -p datafusion --lib` with `-Ztime-passes`, alternated with
the base
so machine drift cancels out:

| | total | `evaluate_obligation` |
|---|---|---|
| base | 88.9s | 75.0s |
| after first commit | 18.6s | 11.15s |
| after second commit | **8.2s** | **234ms** |

234ms over 34,724 goals is 6.7µs each — the same rate as goals that
carry an
empty `ParamEnv`, i.e. the repeated proving is gone rather than merely
reduced.
What remains in this crate is LLVM: 7.6s emitting objects and 4.2s in
LLVM
passes.

An earlier interleaved wall-clock A/B of the first commit alone measured
74.4s/69.9s base against 16.6s/15.5s fixed.

## Are there any user-facing changes?

No, other than the relaxed `Self: Sync` bound described above. No public
signature changes — after macro expansion these methods have the same
signatures
as before.


🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
oc7o pushed a commit to oc7o/datafusion that referenced this pull request Aug 13, 2026
…e#24338)

## Which issue does this PR close?

Adresses: apache#13814

Found while profiling compile times for apache#24325 / apache#24326 / apache#24329 /
apache#24330.

## Rationale for this change

`datafusion/core/src/bin/` holds three binaries that regenerate the docs
under
`docs/source/user-guide`: `print_config_docs`,
`print_runtime_config_docs` and
`print_functions_docs`. Cargo auto-discovers them and they have no
`required-features`, so **every `cargo build` links all three** — each
one
~174MB, since each links the whole `datafusion` rlib.

Nothing in normal development uses them. They are run by
`dev/update_config_docs.sh` and `dev/update_function_docs.sh`, and by
the CI job
that checks the committed docs are up to date.

Two places where this shows up:

**Cold builds.** The three binaries link *after* every other unit has
finished,
so they sit on the critical path with nothing to overlap with.
`cargo build --timings` shows them occupying the last **3.5s** of a
`cargo build -p datafusion` (~8.8s of CPU), after the last library unit
completes.

**The tightest inner loop** — touch a file in core, rebuild. All three
are
relinked every time:

```
before: 3.0s  2.4s
after:  1.3s  1.1s
```

## What changes are included in this PR?

The three binaries move behind a new non-default `docs_generation`
feature, and
the two `dev/` scripts pass `--features docs_generation`.

Using `required-features` means declaring the `[[bin]]` targets
explicitly, since
auto-discovered targets cannot carry it.

## Are these changes tested?

- `cargo build -p datafusion` no longer produces the three binaries
- `cargo build -p datafusion --features docs_generation` does
- `./dev/update_config_docs.sh` still regenerates
  `docs/source/user-guide/configs.md` byte-identically (empty `git diff`
  afterwards), which is what the CI doc check compares

`dev/update_function_docs.sh` uses the same invocation pattern and all
three of
its call sites were updated; CI exercises both scripts.

## Are there any user-facing changes?

The three binaries are no longer built by a default `cargo build`.
Anyone who
ran them directly needs `--features docs_generation` — same as the
`dev/` scripts
now do. No library API changes.

If you would rather these lived outside the published crate altogether,
moving
them to a small non-published `dev/` crate would have the same effect on
build
times; I went with the smaller change.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

catalog Related to the catalog crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants