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

API proposal: Add SpinWait.SpinOnce overload to specify or disable the Sleep(1) threshold #26132

Closed
kouvel opened this issue May 10, 2018 · 3 comments
Labels
api-approved API was approved in API review, it can be implemented area-System.Threading
Milestone

Comments

@kouvel
Copy link
Member

kouvel commented May 10, 2018

SpinWait.SpinOnce has always used a fixed threshold before which it would start to call Thread.Sleep(1).

The jump in delay from spinning/Sleep(0) to a Sleep(1) is many orders in magnitude, and no single threshold will be good for every spinning usage scenario. An internal overload was added to allow specifying the Sleep(1) threshold in some places where we already encountered this issue:

In some cases, the scenario may also prefer to disable Sleep(1) altogether, for instance in Interlocked.CompareExchange() loops, see https://github.com/dotnet/corefx/issues/29595. Also, when a proper wait follows the spinning, the spin loop typically should not do Sleep(1) as doing the proper wait is usually better, for example:

Proposed API

    public struct SpinWait
    {
        public void SpinOnce(int minimumSleep1SpinCountThreshold) { }
    }

minimumSleep1SpinCountThreshold:

  • < -1: ArgumentOutOfRangeException
  • == -1: Sleep(1) will not be used
  • >= 0: Specifies a minimum spin count after which Sleep(1) may be used based on unspecified heuristics
    • For the implementation, we could probably make the value have a minimum of SpinWait.YieldThreshold such that the value returned by SpinWait.NextSpinWillYield would still be somewhat meaningful
  • Not sure about the name, suggestions?

Alternatives

Add a field for the Sleep(1) threshold to SpinWait and a constructor overload to set it.

  • Effects of increasing its size are unknown and could have undesired side-effects

@stephentoub

@kouvel kouvel self-assigned this May 10, 2018
@terrajobst
Copy link
Member

terrajobst commented May 22, 2018

Video

  • Looks good but we should shorten the name.
  • Are there any other thresholds that we need to expose?
public struct SpinWait
{
    public void SpinOnce(int sleep1Threshold);
}

@kouvel
Copy link
Member Author

kouvel commented May 22, 2018

Thanks, that name looks good to me. I don't think we need to expose anything else at the moment.

kouvel referenced this issue in kouvel/coreclr May 30, 2018
To allow customizing the spin count threshold for Sleep(1) usage, and to allow disabling the use of Sleep(1).

API review: https://github.com/dotnet/corefx/issues/29623
Part of fix for https://github.com/dotnet/corefx/issues/29595
kouvel referenced this issue in kouvel/corefx May 30, 2018
Depends on dotnet/coreclr#18204

To allow customizing the spin count threshold for Sleep(1) usage, and to allow disabling the use of Sleep(1).

API review: https://github.com/dotnet/corefx/issues/29623
Part of fix for https://github.com/dotnet/corefx/issues/29595
@kouvel
Copy link
Member Author

kouvel commented May 31, 2018

Based on discussion in dotnet/coreclr#18204, we decided to change the value of 0 to disable Sleep(1) usage to -1 instead, similar to timeouts where -1 == no/infinite timeout. Edited proposal inline above to reflect this.

@kouvel kouvel removed their assignment Jun 1, 2018
kouvel referenced this issue in kouvel/coreclr Jun 13, 2018
To allow customizing the spin count threshold for Sleep(1) usage, and to allow disabling the use of Sleep(1).

API review: https://github.com/dotnet/corefx/issues/29623
Part of fix for https://github.com/dotnet/corefx/issues/29595
kouvel referenced this issue in kouvel/coreclr Jul 13, 2018
To allow customizing the spin count threshold for Sleep(1) usage, and to allow disabling the use of Sleep(1).

API review: https://github.com/dotnet/corefx/issues/29623
Part of fix for https://github.com/dotnet/corefx/issues/29595
kouvel referenced this issue in kouvel/corefx Jul 13, 2018
Depends on dotnet/coreclr#18204

To allow customizing the spin count threshold for Sleep(1) usage, and to allow disabling the use of Sleep(1).

API review: https://github.com/dotnet/corefx/issues/29623
Part of fix for https://github.com/dotnet/corefx/issues/29595
kouvel referenced this issue in dotnet/coreclr Jul 15, 2018
Expose SpinWait.SpinOnce(int sleep1Threshold) overload

To allow customizing the spin count threshold for Sleep(1) usage, and to allow disabling the use of Sleep(1).

API review: https://github.com/dotnet/corefx/issues/29623
Part of fix for https://github.com/dotnet/corefx/issues/29595
dotnet-maestro-bot referenced this issue in dotnet-maestro-bot/corefx Jul 15, 2018
Expose SpinWait.SpinOnce(int sleep1Threshold) overload

To allow customizing the spin count threshold for Sleep(1) usage, and to allow disabling the use of Sleep(1).

API review: https://github.com/dotnet/corefx/issues/29623
Part of fix for https://github.com/dotnet/corefx/issues/29595

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
dotnet-maestro-bot referenced this issue in dotnet-maestro-bot/corert Jul 15, 2018
Expose SpinWait.SpinOnce(int sleep1Threshold) overload

To allow customizing the spin count threshold for Sleep(1) usage, and to allow disabling the use of Sleep(1).

API review: https://github.com/dotnet/corefx/issues/29623
Part of fix for https://github.com/dotnet/corefx/issues/29595

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
stephentoub referenced this issue in dotnet/corefx Jul 15, 2018
Expose SpinWait.SpinOnce(int sleep1Threshold) overload

To allow customizing the spin count threshold for Sleep(1) usage, and to allow disabling the use of Sleep(1).

API review: https://github.com/dotnet/corefx/issues/29623
Part of fix for https://github.com/dotnet/corefx/issues/29595

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
jkotas referenced this issue in dotnet/corert Jul 15, 2018
Expose SpinWait.SpinOnce(int sleep1Threshold) overload

To allow customizing the spin count threshold for Sleep(1) usage, and to allow disabling the use of Sleep(1).

API review: https://github.com/dotnet/corefx/issues/29623
Part of fix for https://github.com/dotnet/corefx/issues/29595

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
kouvel referenced this issue in dotnet/corefx Jul 25, 2018
Expose SpinWait.SpinOnce(int sleep1Threshold) overload

Depends on dotnet/coreclr#18204

To allow customizing the spin count threshold for Sleep(1) usage, and to allow disabling the use of Sleep(1).

API review: https://github.com/dotnet/corefx/issues/29623
Part of fix for https://github.com/dotnet/corefx/issues/29595
@kouvel kouvel closed this as completed Aug 30, 2018
@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 3.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Threading
Projects
None yet
Development

No branches or pull requests

3 participants