Process cascaded work immediately #427
Conversation
|
/cc @halter73 |
benaadams
force-pushed
the
Process-cascaded-work
branch
from
522cfdf to
4beec24
Compare
| { | ||
| // Queues swapped | ||
| wasWork = DoPostWork(); | ||
| wasWork = DoPostCloseHandle() || wasWork; |
There was a problem hiding this comment.
Why is it important to call the DoPost methods twice in one iteration? I think it would be cleaner to just double _maxLoops.
There was a problem hiding this comment.
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.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Yeah; will think a while and see if the reason comes to me; else revert.
There was a problem hiding this comment.
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 :)
|
I think we need to measure this once #363 is in. |
benaadams
force-pushed
the
Process-cascaded-work
branch
3 times, most recently
from
ea91e1a to
7bd14bd
Compare
benaadams
force-pushed
the
Process-cascaded-work
branch
from
7bd14bd to
d72e6ae
Compare
Without waiting for next libuv pass Fix for potential regression in aspnet#363 due to bug fix.
|
Merged with #363 as is response to that. |
Without waiting for next libuv pass; due to it being added to the
_workAddingbuffer and having to wait to the next pass to move to the_workRunningbuffer to be submitted; which will currently throttle writes to network and handle close speed.This is to get the events into the libuv loop as early as possible as it determines its sleep time between loops based on pending work. Otherwise end up with a throttled message pump.
Fix for potential regression in #363 due to bug fix.