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.

Sign up for GitHub

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

Jump to bottom

Work remaining for notification profilers #54211

Closed
4 of 8 tasks
davmason opened this issue Jun 15, 2021 · 2 comments · Fixed by #57429
Closed
4 of 8 tasks

Work remaining for notification profilers #54211

davmason opened this issue Jun 15, 2021 · 2 comments · Fixed by #57429
Assignees
@davmason
Labels
area-Diagnostics-coreclr
Milestone
Future

Comments

@davmason
Copy link
Member

davmason commented Jun 15, 2021 •
edited
Loading

#53122 introduced the feature, this tracks the remaining work.

Code

  • Introduce code to not check all 32 slots for every callback (I am thinking about keeping a "max used" index and iterating until that is reached)
  • Rename ProfControlBlock::FindNextFreeProfilerInfoSlot to indicate that it has side effects and isn't just a getter
  • Potential rename to something besides notification profilers, since they can technically have side effects

Testing

  • Stress test profilers attaching and detaching
  • Run all runtime tests with notification profiler attached
  • Test multiple profilers each with an eventpipe session

Documentation

  • Write description of feature
  • Write and publish sample to dotnet/samples
@davmason davmason added this to the 6.0.0 milestone Jun 15, 2021
@davmason davmason self-assigned this Jun 15, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Jun 15, 2021
@ghost
Copy link

ghost commented Jun 15, 2021

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

Issue Details

#53122 introduced the feature, this tracks the remaining work.

Code

  • Introduce code to not check all 32 slots for every callback (I am thinking about keeping a "max used" index and iterating until that is reached)
  • Rename ProfControlBlock::FindNextFreeProfilerInfoSlot to indicate that it has side effects and isn't just a getter

Testing

  • Stress test profilers attaching and detaching
  • Run all runtime tests with notification profiler attached
  • Test multiple profilers each with an eventpipe session

Documentation

  • Write description of feature
  • Write and publish sample to dotnet/samples
Author: davmason
Assignees: davmason
Labels:

area-Diagnostics-coreclr
Milestone: 6.0.0

@davmason davmason removed the untriaged New issue has not been triaged by the area owner label Jun 15, 2021
@hoyosjs hoyosjs linked a pull request Aug 16, 2021 that will close this issue
Add tests for notification profilers #57429
Merged
@davmason davmason modified the milestones: 6.0.0, Future Aug 18, 2021
@davmason
Copy link
Member Author

This is a meta issue, the testing part that needed done for 6 is done so I am moving it out to future. It still tracks documentation

@ghost ghost locked as resolved and limited conversation to collaborators Sep 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@davmason davmason

Labels
area-Diagnostics-coreclr
Projects
None yet
Milestone
Future
Development

Successfully merging a pull request may close this issue.

Add tests for notification profilers davmason/runtime
1 participant
@davmason