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

Report POH in ETW events #34549

Merged
7 commits merged into from
Apr 9, 2020
Merged

Report POH in ETW events #34549

7 commits merged into from
Apr 9, 2020

Conversation

VSadov
Copy link
Member

@VSadov VSadov commented Apr 4, 2020

Follow-up item for: #13731

=== When existing events have a way to specify generation, just report POH.

  • GCCreateSegment_V1
  • GCAllocationTick_V3
  • GCGenerationRange (changes in descr_generations_to_profiler)

=== Extend existing events to fit POH.

  • BGCOverflow
  • GCHeapStats_V1

@VSadov VSadov changed the title [WIP] Report POH in ETW events Report POH in ETW events Apr 6, 2020
@VSadov VSadov marked this pull request as ready for review April 7, 2020 02:11
@VSadov VSadov requested a review from Maoni0 April 7, 2020 02:11
@VSadov
Copy link
Member Author

VSadov commented Apr 7, 2020

Made new ETW event versions backwards compatible. Now this is ready for review.

@VSadov
Copy link
Member Author

VSadov commented Apr 7, 2020

Created an issue in perfview repo to add support for this on PerfView side once this PR is merged. - microsoft/perfview#1146

{
uint8_t* address = heap_segment_mem (seg);
size_t size = heap_segment_reserved (seg) - heap_segment_mem (seg);
gc_etw_segment_type segment_type = (i == 3) ?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gc_etw_segment_type segment_type = (i == 3) ? [](start = 15, length = 46)

why not do (i == loh_generation) here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just did not think about it. It does seem odd.

Copy link
Member

@Maoni0 Maoni0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the rest LGTM! do you also have the change in TraceEvent for this (to test)?

Copy link
Member

@Maoni0 Maoni0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@VSadov
Copy link
Member Author

VSadov commented Apr 9, 2020

Thanks!!!

@ghost
Copy link

ghost commented Apr 9, 2020

Hello @VSadov!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants