Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

SpinLock.TryEnter fail fast for timeout 0 #6952

Merged
merged 3 commits into from
Oct 14, 2016
Merged

SpinLock.TryEnter fail fast for timeout 0 #6952

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8efee70
SpinLock.TryEnter fail fast for timeout 0 (2)
benaadams Aug 27, 2016
f3f9314
Don't over check SpinLock timeout
benaadams Aug 27, 2016
b9df723
check millisecondsTimeout outside critical region
benaadams Oct 14, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 18 additions & 17 deletions src/mscorlib/src/System/Threading/SpinLock.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,30 +339,30 @@ private void ContinueTryEnter(int millisecondsTimeout, ref bool lockTaken)

if (Interlocked.CompareExchange(ref m_owner, observedOwner | 1, observedOwner, ref lockTaken) == observedOwner)
{
// Aquired lock
return;
}

#if !FEATURE_CORECLR
Thread.EndCriticalRegion();
#endif
if (millisecondsTimeout == 0)
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to check for millisecondsTimeout in TryEnter as well - right before ContinueTryEnter?

Copy link
Member Author

@benaadams benaadams Oct 14, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Probably rather than attempting a double acquire. It would mean adding another Thread.EndCriticalRegion() on a not acquire branch.

However its a pretty heavily stacked function which does currently always inline; maybe revisit as an enhancement? (i.e. would need some balancing to get right and keep it inlining)

Copy link
Member

Choose a reason for hiding this comment

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

Make sense.

{
// Did not aquire lock in CompareExchange and timeout is 0 so fail fast
return;
}
}
else if (millisecondsTimeout == 0)
{
// Did not aquire lock as owned and timeout is 0 so fail fast
return;
}
else //failed to acquire the lock,then try to update the waiters. If the waiters count reached the maximum, jsut break the loop to avoid overflow
{
if ((observedOwner & WAITERS_MASK) != MAXIMUM_WAITERS)
turn = (Interlocked.Add(ref m_owner, 2) & WAITERS_MASK) >> 1 ;
}



// Check the timeout.
if (millisecondsTimeout == 0 ||
(millisecondsTimeout != Timeout.Infinite &&
TimeoutHelper.UpdateTimeOut(startTime, millisecondsTimeout) <= 0))
{
DecrementWaiters();
return;
}

//***Step 2. Spinning
//lock acquired failed and waiters updated
int processorCount = PlatformHelper.ProcessorCount;
Expand Down Expand Up @@ -396,15 +396,16 @@ private void ContinueTryEnter(int millisecondsTimeout, ref bool lockTaken)
#endif
}
}
}

// Check the timeout.
if (millisecondsTimeout != Timeout.Infinite && TimeoutHelper.UpdateTimeOut(startTime, millisecondsTimeout) <= 0)
{
DecrementWaiters();
return;
// Check the timeout.
if (millisecondsTimeout != Timeout.Infinite && TimeoutHelper.UpdateTimeOut(startTime, millisecondsTimeout) <= 0)
{
DecrementWaiters();
return;
}
}


//*** Step 3, Yielding
//Sleep(1) every 50 yields
int yieldsoFar = 0;
Expand Down