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

Merged
merged 5 commits into from
Oct 18, 2024
Merged

Fix tracy #4500

merged 5 commits into from
Oct 18, 2024

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.

@graydon graydon added this pull request to the merge queue Oct 17, 2024
@graydon graydon removed this pull request from the merge queue due to a manual request Oct 17, 2024
@graydon graydon added this pull request to the merge queue Oct 18, 2024
Merged via the queue into stellar:master with commit 2493152 Oct 18, 2024
26 checks passed
@graydon graydon deleted the fix-tracy branch October 18, 2024 04:56
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