Skip to content
This repository was archived by the owner on Dec 18, 2018. It is now read-only.

Process cascaded work immediately #427

Closed
wants to merge 1 commit into from
Closed

Process cascaded work immediately #427

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
29 changes: 25 additions & 4 deletions src/Microsoft.AspNet.Server.Kestrel/Infrastructure/KestrelThread.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ namespace Microsoft.AspNet.Server.Kestrel
/// </summary>
public class KestrelThread
{
// maximum times the work queues swapped and are processed in a single pass
// as completing a task may immediately have write data to put on the network
// otherwise it needs to wait till the next pass of the libuv loop
private const int _maxLoops = 8;

private static Action<object, object> _threadCallbackAdapter = (callback, state) => ((Action<KestrelThread>)callback).Invoke((KestrelThread)state);
private KestrelEngine _engine;
private readonly IApplicationLifetime _appLifetime;
Expand Down Expand Up @@ -249,11 +254,17 @@ private void ThreadStart(object parameter)

private void OnPost()
{
DoPostWork();
DoPostCloseHandle();
var loopsRemaining = _maxLoops;
bool wasWork;
do
{
wasWork = DoPostWork();
wasWork = DoPostCloseHandle() || wasWork;
Copy link
Member

Choose a reason for hiding this comment

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

Why is it important to call the DoPost methods twice in one iteration? I think it would be cleaner to just double _maxLoops.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I started with that; then changed to this, didn't add reason :(

I think it was to make sure a minimum of a double loop had always been done and both queues processed in case a task had snuck in; to get the tasks into the Libuv queue as early as possible for lower latency.

libuv-loop

But if it was DoPostWork that added an item to the queue then wasWork should return true so that would be ok as it would do another loop. If wasWork returned false, then DoPostWork and DoPostCloseHandle would both have returned pretty fast so probably wouldn't be anything extra there?

My worry was the libuv loop sleeping (15ms?) because it had nothing to do; however an item sitting in the thread queue ready to go; but missed due to uv_async_send coalescing and blocked on the add behind the lock; but I was probably being over cautious?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think coalescing is an issue since we always call uv_async_send after enqueing to _workAdding. We've verified that calling uv_async_send will not coalesce if it is the first call after the post callback starts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah; will think a while and see if the reason comes to me; else revert.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually have taken a better approach - have reverted; then will submit another PR if I can come up with a good reason backed by metrics :)

loopsRemaining--;
} while (wasWork && loopsRemaining > 0);
}

private void DoPostWork()
private bool DoPostWork()
{
Queue<Work> queue;
lock (_workSync)
Expand All @@ -262,6 +273,9 @@ private void DoPostWork()
_workAdding = _workRunning;
_workRunning = queue;
}

bool wasWork = queue.Count > 0;

while (queue.Count != 0)
{
var work = queue.Dequeue();
Expand All @@ -286,8 +300,10 @@ private void DoPostWork()
}
}
}

return wasWork;
}
private void DoPostCloseHandle()
private bool DoPostCloseHandle()
{
Queue<CloseHandle> queue;
lock (_workSync)
Expand All @@ -296,6 +312,9 @@ private void DoPostCloseHandle()
_closeHandleAdding = _closeHandleRunning;
_closeHandleRunning = queue;
}

bool wasWork = queue.Count > 0;

while (queue.Count != 0)
{
var closeHandle = queue.Dequeue();
Expand All @@ -309,6 +328,8 @@ private void DoPostCloseHandle()
throw;
}
}

return wasWork;
}

private struct Work
Expand Down