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

Include datas in GCSettingsEvent #112022

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cshung
Copy link
Member

@cshung cshung commented Jan 31, 2025

This allow us to know if DATAS is enabled through the trace when the GCSettingsRundown event is enabled.

PerfView support is here

@cshung cshung self-assigned this Jan 31, 2025
Copy link
Contributor

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

@mrsharm
Copy link
Member

mrsharm commented Jan 31, 2025

CC: @Maoni0: We were just discussing this. There are other ways from an event perspective to discern if we are running DATAS e.g., if the SizeAdaptationSample event exists.

@cshung
Copy link
Member Author

cshung commented Jan 31, 2025

CC: @Maoni0: We were just discussing this. There are other ways from an event perspective to discern if we are running DATAS e.g., if the SizeAdaptationSample event exists.

I thought those events are intentionally not exposed through the API so that we reserve the right to alter it?

Meanwhile I figured we might want to add a few more flags (e.g. are we using clrgc?, are we using segments?)

Last but not least, wonder if we need to bump the event version for this, I am not doing this right now for simplicity, it doesn't seem required anywhere.

@mangod9
Copy link
Member

mangod9 commented Jan 31, 2025

I feel we should let it be as is for now. There are other mechanisms to retrieve this info, so I would not prioritize this now.

@cshung
Copy link
Member Author

cshung commented Jan 31, 2025

I feel we should let it be as is for now. There are other mechanisms to retrieve this info, so I would not prioritize this now.

Sure, the change is ready for it, it can always wait until the priority comes.

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

Successfully merging this pull request may close these issues.

3 participants