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

Append tag name to parallel construct name for Profiling #842

Closed
ibaned opened this issue May 26, 2017 · 10 comments
Closed

Append tag name to parallel construct name for Profiling #842

ibaned opened this issue May 26, 2017 · 10 comments
Assignees
Labels
Enhancement Improve existing capability; will potentially require voting
Milestone

Comments

@ibaned
Copy link
Contributor

ibaned commented May 26, 2017

carry-over kokkos/kokkos-tools#14

@stanmoore1

@ibaned ibaned added the Enhancement Improve existing capability; will potentially require voting label May 26, 2017
@ibaned ibaned self-assigned this May 26, 2017
@ibaned
Copy link
Contributor Author

ibaned commented May 26, 2017

Not sure how feasible this is yet, but conceptually it makes sense

@crtrott
Copy link
Member

crtrott commented May 26, 2017

Its something we could do. Need to check how much launch latency it adds. But basically we use already typeid(functor).name() as default label, and we can simply append it with typeid(policy_type::tag_type).name() or so.

@stanmoore1
Copy link
Contributor

If the launch latency is high, then could you only enable it when profiling is also enabled?

@ibaned
Copy link
Contributor Author

ibaned commented May 26, 2017

@stanmoore1 I think thats already the case for the existing code that queries the typeid, anything we add inside that #ifdef block would similarly be protected.

@nmhamster
Copy link
Contributor

The issue here is that we want to have profiling be enabled by default. We may end up with some functions for profiling disabled if they affect performance but the premise is "always available profiling". We can talk offline as to why this is important.

@ibaned
Copy link
Contributor Author

ibaned commented May 26, 2017

I created PR #846, which I think will do what you want. I guess we could do some kind of performance testing with LAMMPS to see how it affects latency ?

@crtrott
Copy link
Member

crtrott commented May 26, 2017

I think it should be ok. That said, all pull requests are on hold until:
(i) integration is done
(ii) Dan (Sunderlands) stuff is in.

ibaned added a commit that referenced this issue Jun 7, 2017
Incorporate tags into profiling names [#842]
@ibaned
Copy link
Contributor Author

ibaned commented Jun 7, 2017

The changes are in develop

@stanmoore1
Copy link
Contributor

@ibaned thanks!

@ibaned ibaned modified the milestone: 2017-June-end Jun 14, 2017
@crtrott crtrott closed this as completed Jul 27, 2017
@stanmoore1
Copy link
Contributor

Just wanted to say thanks again for this enhancement, it makes profiling my code a lot easier

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Improve existing capability; will potentially require voting
Projects
None yet
Development

No branches or pull requests

4 participants