Skip to content

Conversation

@2010YOUY01
Copy link
Contributor

Which issue does this PR close?

  • Closes #.

Rationale for this change

release-nonlto build produces binaries with performance close to --release build, but compiles faster. This compilation profile is used for easier development and benchmarking.
Now it by default strips the debug symbols, as a consequence, If a flamegraph is collected using this release-nonlto build, useful function names will not displayed.

Reproducer:

sudo cargo flamegraph --profile release-nonlto --bin dfbench -- sort-tpch -p /Users/yongting/Code/datafusion/benchmarks/data/tpch_sf10 -q 3
The generated flamegraph will look like:
image

If we keep the symbol names, the flamegraph will be generated with function names in the call stack.
image

What changes are included in this PR?

Changed cargo.toml to keep debug symbols in release-nonlto build.

Are these changes tested?

Tested manually in the above example.

Are there any user-facing changes?

No.

@Omega359
Copy link
Contributor

I personally just added a 'profiling' profile that inherits release-nonlto and set debug to true.

Copy link
Contributor

@zhuqi-lucas zhuqi-lucas left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

I test it in my mac m4, it works for me.

Copy link
Contributor

@comphead comphead left a comment

Choose a reason for hiding this comment

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

Thanks @2010YOUY01 that makes for non LTO release to keep symbols to get the debugging.

@comphead
Copy link
Contributor

I personally just added a 'profiling' profile that inherits release-nonlto and set debug to true.

thats another option yes, release-nonlto is a release-like used for running tests to save some time on compilation(because of no LTO) and tests are too small to get benefits from LTO applied.

A small concern about local profile is having deals with conflicts every time the toml file is updated

@comphead comphead merged commit 635e73b into apache:main Mar 21, 2025
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants