Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix tracy #4500

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Fix tracy #4500

wants to merge 5 commits into from

Conversation

graydon
Copy link
Contributor

@graydon graydon commented Oct 6, 2024

This change returns tracy to kinda-working on soroban. It does not work on soroban v21 (you still just get an opaque span for such InvokeHostFunction calls) but does:

  • Build for soroban v22
  • Even when running with CXX=clang++-12 CXXFLAGS=-stdlib=libc++ as in CI
  • Work for soroban v22 (not-crash and actually produce tracy sub-spans for soroban v22)
  • Roll back the stale cost-adjustments made for SOROBAN_TEST_EXTRA_PROTOCOL=22 mode
  • As a bonus, simplify lifetime/startup and eliminate a layer of locking in the memory-tracking code

There were three major issues to deal with:

  • We weren't passing relevant toolchain variables from the Makefile through to cargo invocations, at all.
  • We weren't setting CXXSTDLIB which is a special one that build.rs needs to build C++ code.
  • We were using tracy in manual-lifetime mode because older versions of the rust tracy-client crate required it, and this just can't really work when there are multiple copies of tracy-client linked into the final executable, they fight over who gets to initialize tracy.

By abandoning those old versions of tracy-client and only enabling new ones that allow turning off manual-lifetime, we're in a much better state. The multiple copies of tracy-client all expect tracy to initialize itself, and it does so, and everything's fine.

I did not yet conditionalize-out the memory-tracking code. I might push a further commit here to do so. It seems like it's fairly expensive to have on by default when using tracy, and I'm not sure anyone benefits from it most of the time.

@sisuresh sisuresh closed this Oct 10, 2024
@graydon graydon reopened this Oct 15, 2024
@graydon
Copy link
Contributor Author

graydon commented Oct 15, 2024

This was accidentally closed.

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.

2 participants