search

aclysma / profiling

Provides a very thin abstraction over instrumented profiling crates like puffin, optick, tracy, and superluminal-perf.
Apache License 2.0
324 stars 41 forks source link

Bump tracy-client to 0.15 #46

Closed Imberflur closed 1 year ago

Imberflur commented 1 year ago

Supercedes https://github.com/aclysma/profiling/pull/44

Imberflur commented 1 year ago

@aclysma are there any blockers here?

I believe this addresses the first point in https://github.com/aclysma/profiling/pull/44#issuecomment-1356942468

For the second one you mention:

It's annoying that the "correct" version of tracy to use will not be well-defined. But I don't think we can do anything about it from this crate.

So I assume nothing needs to be done here.

I think this crate could specify a specific version via a direct dependency on tracy-client-sys, but that might be a bit opinionated (although the end user can always patch in their own version of profiling if they really need to use an older version of tracy...).

The CI failure seems to be an unrelated clippy warning in the example code.

aclysma commented 1 year ago

I didn't merge the PR because it failed CI. However looking again, I think the change I made in https://github.com/aclysma/profiling/commit/5f3fe75cd0426afd2bd0213ef8f20706fbc20e1f probably fixes this issue. I don't think I can force CI to re-run so I'll see if I can make another PR from your branch just to check that it is working.

aclysma commented 1 year ago

I pushed your branch onto this repo and rebased it over latest to see if it builds: https://github.com/aclysma/profiling/tree/tracy-15

aclysma commented 1 year ago

Looks like it builds fine, do you want to rebase your branch so that it passes CI or should I just merge this branch? https://github.com/aclysma/profiling/tree/tracy-15

Imberflur commented 1 year ago

@aclysma just merging that branch sounds good to me.

aclysma commented 1 year ago

Will merge the other branch in #51