Dandandan opened a new pull request, #24325:
URL: https://github.com/apache/datafusion/pull/24325
## Which issue does this PR close?
None yet — happy to open one if useful.
## 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`),
and it
sits *alone* on the critical path between `datafusion-session` and
`datafusion-catalog-listing`. That is ~11ms per line, against
0.09–0.55ms/line
for crates like `datafusion-datasource` or `datafusion-physical-plan`.
`-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 |
The mechanism: `#[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 pulls in the whole `Expr`/`LogicalPlan` type graph
(~500 types) through `Expr::{Exists, InSubquery, ScalarSubquery}`.
Two measurements that pin this down:
- 16,440 `Send`/`Sync` goals over only **1,250 distinct** `(trait, type)`
pairs
— a mean of 13× redundant re-proving.
- Goals carrying a non-empty `ParamEnv` cost **~1.6ms** each; the same goals
with an empty `ParamEnv` cost **~6µs** — 270× cheaper.
Seven `#[async_trait]` impls accounted for 51.7s of the crate's 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 seven 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.
Method bodies are moved verbatim into inherent `async fn`s. Nothing else
changes: the returned type is still `BoxFuture` (`+ Send`), and futures are
still lazy — `Box::pin(self.m_inner(..))` does not poll anything.
Isolated probe confirming the shape 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?
Existing tests: `cargo test -p datafusion-catalog` (9 passed) and
`cargo test -p datafusion --lib` (442 passed). 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: 20.5s 19.9s 18.3s
after: 4.8s 4.5s 3.9s
```
`evaluate_obligation` drops from 8.05s to 3.39s, and its goal count from
25,811
to 16,647. In a full `cargo build -p datafusion` the unit goes from 42.3s to
~8s.
## Are there any user-facing changes?
No. No public signature changes — after macro expansion the trait methods
have
the same signatures as before.
### Follow-ups
The same pattern applies to the rest of the serial tail of the build, which
is
where the remaining time is:
| crate | frontend | source lines | ms/line |
|---|---|---|---|
| `datafusion` (core) | 74.5s | 33,144 | 2.25 |
| `datafusion-catalog-listing` | 20.5s | 3,013 | 6.81 |
| `datafusion-session` | 18.8s | 1,619 | 11.59 |
A more general fix would be to drop `#[async_trait]` from these traits in
favour
of an explicit `BoxFuture` return type with a single lifetime and no
where-clauses. That would make *every* impl cheap, including in downstream
crates, but it is a breaking change to a public trait, so I have left it out
here.
Also worth noting separately: this workspace's `dev` profile has incremental
compilation on, and for this trait-solving-heavy workload it costs 1.8× more
CPU
on a cold build (452.6s → 249.9s of CPU across the DataFusion crates with
`CARGO_INCREMENTAL=0`). The `ci` profile already sets `incremental = false`.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
