Repeating this comment from #1119 (comment):
After taking a step back from this and considering that we are working on support for CancellationTokens, I wonder if we should consider removing LimitedConcurrencyLevelTaskScheduler.Shutdown() and always use CancellationToken instead?
If we do keep the LimitedConcurrencyLevelTaskScheduler.Shutdown() method, there are some things to consider:
- Should we make
LimitedConcurrencyLevelTaskScheduler public? The BCL doesn't have one and it is provided in the docs for TaskScheduler. But there is no ShutDown() on that implementation.
- There are some tests where
Shutdown() was called in Lucene that would likely perform better if we use it, since those lines were often commented out in Lucene.NET. However, using CancellationToken could be an alternative way to handle those cases.
The upside of using Shutdown() is that it aligns more closely to Lucene.
The upside of using CancellationToken is that it is more fine-grained and can potentially shut down a process quicker, since it will be able to intervene inside of the running task instead of prior to queuing the task.
Allowing a user-defined TaskScheduler still seems like a good idea, but since Microsoft didn't allow them to be cancelled, it seems like we should follow the CancellationToken approach instead of trying to shoehorn a way to cancel a TaskScheduler. Using the same CancellationToken to make LimitedConcurrencyLevelTaskScheduler stop queuing new work is also something that might be worth considering.
Originally posted by @NightOwl888 in #1080 (comment)
Repeating this comment from #1119 (comment):
After taking a step back from this and considering that we are working on support for
CancellationTokens, I wonder if we should consider removingLimitedConcurrencyLevelTaskScheduler.Shutdown()and always useCancellationTokeninstead?If we do keep the
LimitedConcurrencyLevelTaskScheduler.Shutdown()method, there are some things to consider:LimitedConcurrencyLevelTaskSchedulerpublic? The BCL doesn't have one and it is provided in the docs forTaskScheduler. But there is noShutDown()on that implementation.Shutdown()was called in Lucene that would likely perform better if we use it, since those lines were often commented out in Lucene.NET. However, usingCancellationTokencould be an alternative way to handle those cases.The upside of using
Shutdown()is that it aligns more closely to Lucene.The upside of using
CancellationTokenis that it is more fine-grained and can potentially shut down a process quicker, since it will be able to intervene inside of the running task instead of prior to queuing the task.Allowing a user-defined
TaskSchedulerstill seems like a good idea, but since Microsoft didn't allow them to be cancelled, it seems like we should follow theCancellationTokenapproach instead of trying to shoehorn a way to cancel aTaskScheduler. Using the sameCancellationTokento makeLimitedConcurrencyLevelTaskSchedulerstop queuing new work is also something that might be worth considering.Originally posted by @NightOwl888 in #1080 (comment)