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

Scope colors #19

Open
kvark opened this issue Apr 27, 2021 · 4 comments
Open

Scope colors #19

kvark opened this issue Apr 27, 2021 · 4 comments
Labels

Comments

@kvark
Copy link

kvark commented Apr 27, 2021

Would it be reasonable to expose the colors in this crate?
It would enhance the tracy backend experience quite a bit. For example, in https://github.com/gfx-rs/wgpu-rs/discussions/879 we would mark every crate (gfx/wgpu/wgpu-rs) with a separate color.

https://github.com/davidwed/tracy#marking-zones describes the way to pass down colors in Tracy.

@aclysma
Copy link
Owner

aclysma commented Apr 27, 2021

I like the feature idea and I would use it myself. That said, I do have similar feelings about this as another feature request from before: #3 (comment)

  • Not all backends support this
  • Some people won't take this crate as a dependency unless it stays very small and simple (the value this crate provides is greater if more crates use it)
  • End-users can interface directly with their profiling crate of choice, so it's still possible for end-users to do this, even if this crate doesn't directly support it.

If we did it, what do you think the API should look like? Two approaches come to mind:

  • Optionally pass a color into any scope!() or function!() procmacro
  • Have some sort of push/pop mechanism using thread locals

The first approach is repetitive, but the second requires using thread locals which aren't free or async-friendly. Also, practically speaking, if multiple upstream crates hardcode their own colors for things, it might make the coloring less useful. Pushing/popping colors might help with this.

@kvark
Copy link
Author

kvark commented Apr 27, 2021

I understand the concern here and fully agree on it, in general.
In particular though, the API complexity increase appears to be small, and the backends that don't understand colors can just ignore it. So it seems feasible to introduce.

If we did it, what do you think the API should look like?

We need a mini-investigation on how the backends expose this, unless you already know :)
If we are talking about Tracy, then your first suggestion makes sense (just an extra optional parameter to the scoping macros).

@aclysma
Copy link
Owner

aclysma commented Apr 29, 2021

It is probably possible to PR improvements to puffin and optick.

We would still need to decide on what the profiling API would look like:

  • Optionally take a color in every scope
  • Push/pop colors
  • Some sort of category system (inspired by optick)
    • This could potentially allow a top-level binary crate to configure colors for upstream libraries. The lookup would likely be a hashmap lookup the first time a scope is invoked. This could be an optional feature. It may be possible to have the category default to the crate name, and suggest a convention of [crate-name]::[category-name] for crates that want to define more. I don't know to what extent this can be automated within a macro

I don't plan to work on this myself at this time, but leaving it here for anyone who wants to pick it up. Happy to review PRs.

@aclysma aclysma changed the title Marker colors (for Tracy) Scope colors Apr 29, 2021
@aclysma aclysma added the idea label Apr 29, 2021
@kvark
Copy link
Author

kvark commented Apr 29, 2021

Thank you for the investigation!

I don't see how (2) push/pop approach is going to work. I want to have different layers of the stack using different colors, so it would force me to do a push -> marker -> pop at each stage, which is a waste. I.e. I will not be able to keep the pushed color for more than 1 scope anyway.

The approach with categories (3) sounds very interesting. I like the idea, but I imagine that it may not work out.
Both (1) and (3) would work for us.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants