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

Fixed the yp_spin_count_unit to be a factor of the original_spin_count_unit rather than a continually increasing value #68879

Merged
merged 3 commits into from
May 5, 2022

Conversation

mrsharm
Copy link
Member

@mrsharm mrsharm commented May 4, 2022

Summary

This PR is to address #68839 where it's clear that SetYieldProcessorScalingFactor is called multiple times thereby increasing the value of yp_spin_count_unit and as a result the spin counts are monotonically increasing each time SetYieldProcessorScalingFactor is called; this function was previously assumed to be called only once but that changed with the following PR: #55295.

The fix is to store the original spin count unit and use that value to set the yp_spin_count_unit rather than adjusting the value based on the last yp_spin_count_unit.

…t_unit rather than a continually increasing value
@ghost
Copy link

ghost commented May 4, 2022

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

Issue Details

Summary

This PR is to address #68839 where it's clear that SetYieldProcessorScalingFactor is called multiple times thereby increasing the value of yp_spin_count_unit and as a result the spin counts are monotonically increasing each time SetYieldProcessorScalingFactor is called; this function was previously assumed to be called only once but that changed with the following PR: #55295.

The fix is to store the original spin count unit and use that value to set the yp_spin_count_unit rather than adjusting the value based on the last yp_spin_count_unit.

Author: mrsharm
Assignees: -
Labels:

area-GC-coreclr

Milestone: -

@Maoni0
Copy link
Member

Maoni0 commented May 5, 2022

I would put a cap on the check too 'cause we don't want to spin too much -

if ((yp_spin_count_unit == 0) || (yp_spin_count_unit > 32768))

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!

@kouvel
Copy link
Member

kouvel commented May 5, 2022

Ah I seem to have missed this issue in my other change. Thanks for fixing it @mrsharm!

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