Dandandan opened a new pull request, #24326:
URL: https://github.com/apache/datafusion/pull/24326
## Which issue does this PR close?
Follow-up to #24325 (same root cause, different shape). Independent of it —
the
two touch different crates.
## Rationale for this change
`datafusion-session` is 1,619 lines of source and spends **18.8s** in the
frontend during a cold `cargo build -p datafusion` (`cargo build --timings`)
—
11.6ms per line, the worst ratio in the workspace, against 0.09–0.55ms/line
for
crates like `datafusion-datasource` or `datafusion-physical-plan`.
`-Zself-profile` puts ~72% of the crate's compile time in
`evaluate_obligation`,
and 98% of that in `Send`/`Sync`:
| trait | time |
|---|---|
| `Send` | 2.29s |
| `Sync` | 1.99s |
| everything else | 0.03s |
Cause is the one from #24325: `#[async_trait]` gives each `async fn` a
`where 'life0: 'async_trait, .., Self: '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. The proof for
everything the future captures is therefore redone per method.
What differs here is *where* it happens. Grouping the goals by the `Self`
type in
their `ParamEnv` shows the cost is not in impls at all:
| `Self` in `ParamEnv` | time | goals |
|---|---|---|
| `Self/#0` (generic — a trait declaration) | 4.30s | 2757 |
| `UnsupportedQueryPlanner` | 0.00s | 71 |
| *(empty `ParamEnv`)* | 0.02s | 596 |
A generic `Self` means these are `async fn`s **with default bodies**,
compiled
once in the trait declaration. Note the last row: the same kind of goals cost
~6µs each with an empty `ParamEnv` versus ~1.6ms with a non-empty one.
## What changes are included in this PR?
Seven default bodies are one-line stubs that nevertheless built a coroutine
capturing `Expr` / `Vec<Expr>` / `&LogicalPlan` / `&TableScan`, each of which
drags in the whole `LogicalPlan` type graph:
- `TableProvider::{insert_into, delete_from, update, truncate, merge_into}`
- `ExtensionPlanner::plan_table_scan`
- `QueryPlanner::create_physical_plan` for `UnsupportedQueryPlanner`
They are now written as the desugaring of `async fn` returning `ready(..)`.
No
coroutine is created, so nothing expensive is captured and there is no
auto-trait work to do. The signatures are exactly what `#[async_trait]`
generates — verified against `-Zunpretty=expanded` output of this crate — so
implementors are unaffected.
`#[async_trait]` on `impl QueryPlanner for UnsupportedQueryPlanner` is
dropped
since that impl no longer contains an `async fn`; the trait declarations
keep it.
`scan_with_args` is deliberately 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 `Vec` cannot outlive
a
hoisted future. `SchemaProvider::table_type` is also left alone — it captures
only `&str` and a boxed future, so it is already cheap.
## Are these changes tested?
Existing tests: `cargo test -p datafusion --lib` (442 passed). Since this
touches
the default methods of a widely implemented public trait, I also ran
`cargo check --all-targets` over the other crates holding implementors:
`datafusion-ffi`, `datafusion-functions-table`, `datafusion-catalog-listing`,
`datafusion-sqllogictest` — all clean, as is
`cargo clippy -p datafusion-session --all-targets`.
Interleaved A/B of `cargo rustc -p datafusion-session --lib`, alternating 3
times
so machine drift cancels out:
```
before: 1.95s 2.27s 1.89s
after: 1.21s 1.57s 1.04s
```
`evaluate_obligation` under identical flags drops from 2.27s to 0.76s (−66%),
and its goal count from 3,424 to 2,492.
Those are standalone-build numbers. In the feature-unified build this crate's
frontend is 18.8s rather than ~2s (each obligation is several times dearer
there), so the absolute saving on a real build should be larger — I have not
measured that directly, since isolating one crate's unit in a full build is
hard
to do without confounding it with machine drift.
## Are there any user-facing changes?
No public signature changes — after macro expansion the trait methods have
the
same signatures as before, so existing implementors and callers are
unaffected.
One behavioural nuance worth flagging for review: these stubs now build their
error eagerly when the method is called rather than on first poll. Since the
bodies are `not_impl_err!(..)` / `Ok(None)` with no side effects, and the
value
is still only observed by polling, this is not observable in practice — but
it
does mean `self.table_type()` is called at call time in the messages that
use it.
🤖 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]
