andygrove opened a new issue, #6210:
URL: https://github.com/apache/datafusion-comet/issues/6210
### What is the problem the feature request solves?
`native/Cargo.toml` sets `lto = "thin"` for the release profile, but
`libcomet` is never built with LTO. `native/core` declares `crate-type =
["cdylib", "rlib"]`, where the rlib is for the criterion benchmarks, and Cargo
only runs LTO for a unit when every one of its crate types supports it. An rlib
does not, so Cargo drops the setting without a warning. The `rustc` line from
`cargo build --release -v -p datafusion-comet` has no `-C lto`:
```
rustc --crate-name comet --edition=2021 core/src/lib.rs ... --crate-type
cdylib --crate-type rlib --emit=dep-info,link -C opt-level=3 -C codegen-units=1
-C debuginfo=2 -C split-debuginfo=unpacked ... -C strip=debuginfo ...
```
`rustc` itself refuses `-C lto` with an rlib crate type ("lto can only be
run for executables, cdylibs and static library outputs"). The release native
libraries come from `make core-amd64-libs` and `make core-arm64-libs`, which
run plain `cargo build --release`, so the published `libcomet` builds don't get
LTO either.
The rlib also keeps symbols exported that a cdylib-only build would
internalize. That is why each thread-local the allocation accounting wrapper
touches costs its own `__tls_get_addr` call on x86_64 Linux: exported
thread-locals use the general-dynamic TLS model, while internal ones share one
call per function under local-dynamic. See the discussion on #6166.
### Describe the potential solution
Measure it first. `cargo rustc --release -p datafusion-comet --lib
--crate-type cdylib` overrides the crate types for that one build, and Cargo
then passes `-C lto=thin`, so an LTO build of `libcomet` can be compared with
the current one on TPC-H without changing the manifest. The build time matters
too, since thin LTO runs over the whole dependency graph.
If LTO pays off, the release targets could build the library that way, or
the cdylib could move into a small crate of its own that depends on
`datafusion-comet`, leaving the rlib for the benchmarks.
### Additional context
Found while reviewing #6166. A scratch crate with the same `[lib]` crate
types and release profile gets the same flags from Cargo, and with
`--crate-type cdylib` it gets `-C lto=thin`.
--
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]
