Skip to content

perf(catalog): cut datafusion-catalog compile time ~6x - #24325

Merged
Dandandan merged 2 commits into
apache:mainfrom
Dandandan:perf/catalog-compile-time
Aug 13, 2026
Merged

perf(catalog): cut datafusion-catalog compile time ~6x#24325
Dandandan merged 2 commits into
apache:mainfrom
Dandandan:perf/catalog-compile-time

Conversation

@Dandandan

@Dandandan Dandandan commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Adresses: #13814

First of four; see also #24326 (datafusion-session), #24329 (datafusion core)
and #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

@github-actions github-actions Bot added the catalog Related to the catalog crate label Aug 13, 2026
`datafusion-catalog` spends ~90% of its compile time in the trait solver
(`evaluate_obligation`), almost all of it proving `Send`/`Sync`.

`#[async_trait]` rewrites `async fn m(&self, ..)` into a method carrying
`where 'life0: 'async_trait, .., Self: 'async_trait`. Those bounds make 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 every type the returned future captures is redone
from scratch for each `async fn` in each impl. For a `TableProvider` the
captured set includes `&[Expr]`, which drags in the entire `Expr` /
`LogicalPlan` type graph (~500 types) via `Expr::{Exists, InSubquery, ..}`.

Measured on this crate: 16,440 `Send`/`Sync` goals over only 1,250 distinct
(trait, type) pairs, and goals carrying a non-empty `ParamEnv` cost ~1.6ms
each against ~6us for the same goals with an empty one. Seven
`#[async_trait]` impls accounted for 51.7s of the crate's 51.8s of trait
solving.

This constructs the future in a shim with no where-clauses, so those proofs
land in the global cache, and leaves the trait method as a hand-written
desugaring that only forwards (it never creates a coroutine of its own).
Bodies are moved verbatim into inherent fns; the ones with no `.await` are
plain fns wrapped in `ready(..)`, so they build no coroutine at all. No
behaviour change.

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

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

`evaluate_obligation` drops 7.31s -> 0.66s and its goal count 25,811 ->
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 between
`datafusion-session` and `datafusion-catalog-listing`, that time comes
straight off the build's wall clock.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@codecov-commenter

codecov-commenter commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 77.93594% with 62 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.13%. Comparing base (ab12f5e) to head (c7d4714).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
datafusion/catalog/src/cte_worktable.rs 35.71% 36 Missing ⚠️
datafusion/catalog/src/view.rs 0.00% 25 Missing ⚠️
datafusion/catalog/src/stream.rs 98.85% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #24325      +/-   ##
==========================================
- Coverage   81.14%   81.13%   -0.02%     
==========================================
  Files        1112     1112              
  Lines      386933   387477     +544     
  Branches   386933   387477     +544     
==========================================
+ Hits       313967   314367     +400     
- Misses      54476    54596     +120     
- Partials    18490    18514      +24     

☔ 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.

@Dandandan
Dandandan force-pushed the perf/catalog-compile-time branch from 45a8abf to 5766348 Compare August 13, 2026 14:24
@Dandandan Dandandan changed the title perf(catalog): cut datafusion-catalog compile time ~4x perf(catalog): cut datafusion-catalog compile time ~4.6x Aug 13, 2026

@AdamGS AdamGS left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified locally, this is an awesome improvement.

Dandandan added a commit to Dandandan/arrow-datafusion that referenced this pull request Aug 13, 2026
Same cause as apache#24325, in a different shape. `#[async_trait]` gives each
`async fn` a `where 'life0: 'async_trait, .., Self: 'async_trait` clause, 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 returned
future captures is redone per method.

Here the cost is in trait *declarations* rather than impls: the expensive
`ParamEnv`s all have a generic `Self`, i.e. they come from `async fn`s with
default bodies. `TableProvider::{insert_into, delete_from, update, truncate,
merge_into}`, `QueryPlanner::create_physical_plan` for
`UnsupportedQueryPlanner`, and `ExtensionPlanner::plan_table_scan` are all
one-line stubs, but each built a coroutine capturing `Expr` / `Vec<Expr>` /
`&LogicalPlan` / `&TableScan`, which drags in the whole `LogicalPlan` graph.

Write those defaults as the desugaring of `async fn` returning
`ready(..)` instead. No coroutine is created, so nothing expensive is captured.
The signatures are exactly what `#[async_trait]` generates (verified against
`-Zunpretty=expanded`), so implementors are unaffected.

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

    before: 1.95s  2.27s  1.89s
    after:  1.21s  1.57s  1.04s

`evaluate_obligation` under identical flags drops 2.27s -> 0.76s (-66%), and
its goal count 3,424 -> 2,492.

`scan_with_args` is left as an `async fn`: its default body needs an owned
projection (`scan` takes `Option<&Vec<usize>>` while `ScanArgs::projection`
yields `&[usize]`), so the local cannot outlive a hoisted future.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Dandandan

Copy link
Copy Markdown
Contributor Author

Verified locally, this is an awesome improvement.

More to come!

Dandandan added a commit to Dandandan/arrow-datafusion that referenced this pull request Aug 13, 2026
Same cause as apache#24325. `#[async_trait]` gives each `async fn` a
`where 'life0: 'async_trait, .., Self: 'async_trait` clause, 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 returned
future captures is redone per method.

Here the cost is in trait *declarations* rather than impls: grouping the goals
by the `Self` type in their `ParamEnv` shows 4.30s of 4.32s under a generic
`Self`, i.e. in `async fn`s with default bodies.

Three of those defaults -- `TableProvider::{delete_from, update, merge_into}` --
are one-line stubs that nevertheless build a coroutine capturing `Vec<Expr>` /
`Expr`, and proving that coroutine `Send` walks the whole `Expr`/`LogicalPlan`
type graph. They are now written as the desugaring of `async fn` returning
`ready(..)`, so no coroutine is created and there is nothing expensive to
prove. The signatures are exactly what `#[async_trait]` generates (verified
against `-Zunpretty=expanded`), so implementors are unaffected.

Measured per method, by reverting one at a time (`evaluate_obligation` self
time, all-converted baseline 659ms):

    delete_from   1.29s   (+631ms)
    update        1.30s   (+641ms)
    merge_into    1.29s   (+631ms)
    truncate      697ms   (+38ms)
    insert_into   665ms   (~0)

Only the methods that actually take `Expr` matter, so `insert_into`,
`truncate`, and the two `planner.rs` bodies (which measured ~0 as well) are
left as `async fn`.

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

    base: 1.582s  1.576s  1.590s
    fix:  0.933s  0.938s  0.932s

`evaluate_obligation` drops 1.28s -> 0.646s.

`scan_with_args` is left alone too: its default body needs an owned projection
(`scan` takes `Option<&Vec<usize>>` while `ScanArgs::projection` yields
`&[usize]`), so the local cannot outlive a hoisted future.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@comphead comphead left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Dandandan, this is a very interesting finding!

Dandandan added a commit to Dandandan/arrow-datafusion that referenced this pull request Aug 13, 2026
Third and largest instance of the problem from apache#24325 and apache#24326. 76% of this
crate's compile time is the trait solver (`evaluate_obligation`), 75.0s 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 it is -- 14 impls, top 10 = 78%:

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

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

Each of those methods is now the hand-written desugaring of `async fn`, which
only forwards; the coroutine is built in a shim with no where-clauses so its
proofs land in the global cache. The `ReadOptions` family 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`. Bodies are
moved verbatim; ones with no `.await` become plain fns wrapped in `ready(..)`.

Note `DefaultPhysicalPlanner::create_initial_plan` already used exactly this
shape (`-> BoxFuture<'a, _>` plus `Box::pin(async move ..)`), there for
recursion rather than for compile time.

Interleaved A/B of `cargo rustc -p datafusion --lib`:

    base: 74.4s  69.9s
    fix:  16.6s  15.5s

A third pair ran under heavy load from a concurrent build (base 219.8s, fix
32.5s) and is excluded; its ratio was consistent.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Dandandan Dandandan changed the title perf(catalog): cut datafusion-catalog compile time ~4.6x perf(catalog): cut datafusion-catalog compile time ~6x Aug 13, 2026
…cent impls

Re-profiling after the previous commit left 656ms of `evaluate_obligation` in
this crate, nearly all of it in `CteWorkTable::scan_with_args` -- the one method
the earlier pass skipped, because it carries an explicit `'a` and so needed the
desugared signature written by hand rather than generated.

Its body has no `.await` at all, so it needs no coroutine: it is now a plain fn
wrapped in `ready(..)`, and `ScanArgs` is borrowed rather than moved since the
body only reads the projection.

`cargo rustc -p datafusion-catalog --lib`:

    1.843s  after the previous commit   (obligations 656ms)
    1.168s  after this one              (obligations  56ms)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Dandandan
Dandandan force-pushed the perf/catalog-compile-time branch from 80c492d to c7d4714 Compare August 13, 2026 16:58
Dandandan added a commit to Dandandan/arrow-datafusion that referenced this pull request Aug 13, 2026
Same cause as apache#24325. `#[async_trait]` gives each `async fn` a
`where 'life0: 'async_trait, .., Self: 'async_trait` clause, 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 returned
future captures is redone per method.

Here the cost is in trait *declarations* rather than impls: grouping the goals
by the `Self` type in their `ParamEnv` shows 4.30s of 4.32s under a generic
`Self`, i.e. in `async fn`s with default bodies.

Three of those defaults -- `TableProvider::{delete_from, update, merge_into}` --
are one-line stubs that nevertheless build a coroutine capturing `Vec<Expr>` /
`Expr`, and proving that coroutine `Send` walks the whole `Expr`/`LogicalPlan`
type graph. They are now written as the desugaring of `async fn` returning
`ready(..)`, so no coroutine is created and there is nothing expensive to
prove. The signatures are exactly what `#[async_trait]` generates (verified
against `-Zunpretty=expanded`), so implementors are unaffected.

Measured per method, by reverting one at a time (`evaluate_obligation` self
time, all-converted baseline 659ms):

    delete_from   1.29s   (+631ms)
    update        1.30s   (+641ms)
    merge_into    1.29s   (+631ms)
    truncate      697ms   (+38ms)
    insert_into   665ms   (~0)

Only the methods that actually take `Expr` matter, so `insert_into`,
`truncate`, and the two `planner.rs` bodies (which measured ~0 as well) are
left as `async fn`.

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

    base: 1.582s  1.576s  1.590s
    fix:  0.933s  0.938s  0.932s

`evaluate_obligation` drops 1.28s -> 0.646s.

`scan_with_args` is left alone too: its default body needs an owned projection
(`scan` takes `Option<&Vec<usize>>` while `ScanArgs::projection` yields
`&[usize]`), so the local cannot outlive a hoisted future.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Dandandan added a commit to Dandandan/arrow-datafusion that referenced this pull request Aug 13, 2026
…~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 added this pull request to the merge queue Aug 13, 2026
Merged via the queue into apache:main with commit 1a770de Aug 13, 2026
40 checks passed
@Dandandan
Dandandan deleted the perf/catalog-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

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>
ryux1 pushed a commit to ryux1/datafusion that referenced this pull request Aug 13, 2026
…~4.4x (apache#24330)

## Which issue does this PR close?

Adresses: apache#13814

Fourth and last crate in the family from apache#24325 (`datafusion-catalog`),
apache#24326
(`datafusion-session`) and apache#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 apache#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 apache#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](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>
joroKr21 pushed a commit to coralogix/arrow-datafusion that referenced this pull request Aug 13, 2026
…ache#24339)

## Which issue does this PR close?

Adresses: apache#13814

Found while profiling compile times for apache#24325 / apache#24326 / apache#24329 /
apache#24330,
which removed the trait-solving cost from the four crates on the
critical path
and left LLVM as the dominant remaining cost.

## Rationale for this change

`dev` is the profile behind every `cargo build` and `cargo test`, so its
debug
info is generated over and over. `debug = "line-tables-only"` keeps file
and line
numbers — panics and `RUST_BACKTRACE` output stay just as useful — and
drops the
variable-level DWARF that only an interactive debugger consumes.

Measured per crate, **interleaved** with the baseline so machine drift
cancels
out. The flag is passed to the crate under test only, so cached
dependency
artifacts stay valid and nothing else moves between the two
measurements:

| crate | `debug = 2` | `line-tables-only` | |
|---|---|---|---|
| `datafusion-physical-plan` | 8.89s | 7.43s | −16% |
| `datafusion-functions-aggregate` | 5.86s | 4.63s | −21% |
| `datafusion-physical-expr` | 4.57s | 3.59s | −21% |
| `datafusion-functions` | 4.64s | 4.04s | −13% |
| `datafusion-expr` | 4.54s | 3.57s | −21% |
| `datafusion-functions-nested` | 4.37s | 3.06s | −30% |
| `datafusion-optimizer` | 3.91s | 3.12s | −20% |
| `datafusion-common` | 3.73s | 3.10s | −17% |
| `datafusion-sql` | 3.57s | 2.43s | −32% |
| `datafusion-datasource-parquet` | 3.27s | 2.44s | −25% |
| `datafusion-datasource` | 1.90s | 1.42s | −25% |
| `datafusion-physical-optimizer` | 1.22s | 0.99s | −19% |
| **sum** | **50.5s** | **39.8s** | **−21%** |

The saving is codegen-side, as you would expect: `datafusion-catalog`,
which
spends its time in the trait solver rather than in LLVM, moves only 7.4s
→ 7.0s.

Artifacts shrink as well — `libdatafusion_physical_plan.rlib` goes from
**141MB
to 100MB**.

## What changes are included in this PR?

One setting on `[profile.dev]`, plus an update to the profile
documentation block
above it, which currently advertises "full debug info" for `dev`.

## Are these changes tested?


## Are there any user-facing changes?

For anyone stepping through DataFusion in a debugger, local variable
inspection
needs `CARGO_PROFILE_DEV_DEBUG=2 cargo build` (or a local override in
`.cargo/config.toml`); the comment in `Cargo.toml` says so. Everything
else —
panic locations, backtraces, `#[test]` failures — is unchanged.


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

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
}

async fn scan(
fn scan<'life0, 'life1, 'life2, 'life3, 'async_trait>(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is amazing -- I think it would help future readers if we left some comments pointing back at an explanation of why this particular implementation helps compile time

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will make a follow on PR to add comments to the relevant places

AdamGS pushed a commit to sakshichitnis27/datafusion that referenced this pull request Aug 14, 2026
…pache#24362)

## Which issue does this PR close?

- Follow on to apache#24325
- Follow on to apache#24326
- Follow on to apache#24329
- Follow on to apache#24330


## Rationale for this change

@Dandandan came up with a very clever way to improve compile time, see
apache#13814 (comment)

However, the workaround (to make a special future and implement the
async trait manually) is not obvious (at least to me) and I worry it
might get removed in the future by accident, as we don't have any checks
to prevent the regression

Thus I think some comments explaining the pattern and how it would work
will help avoid such a problem

## What changes are included in this PR?

Add comments explaining why the code is done this way

## Are these changes tested?

Comments only

## Are there any user-facing changes?

No
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.

5 participants