-
Notifications
You must be signed in to change notification settings - Fork 783
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
Make TaskCreationOptions.LongRunning advice more explicit #64
Comments
We should show the usage of |
My interpretation of that guidance is that given Was this the intention? |
Yes |
Great writing. So in light of the advice to go w/ good-old plain |
I'm not sure if I completely agree with this advice, in that it's not quite functionally the equivalent of var citiesPointsOfInterest = new [] {
LoadDataset(NewYork).ToArray(), // 250 locations; .ToArray() <-- it's in memory so no task discontinuation on await IO.
LoadDataset(Paris).ToArray(), // 400 locations;
LoadDataset(Moscow).ToArray(), // 100 locations;
};
var bestPathThroughCitiesTasks = citiesPointsOfInterest.Select(city =>
Task.Factory.StartNew(ComputeBestPathThroughPointsOfInterest, // Oh that wonderful The Traveling Salesman Problem...
city, TaskCreationOptions.LongRunning // LongRunning here would create new thread so as not to pressure thread pool.
)).ToArray();
var firstDoneTask = await Task.WhenAny(bestPathThroughCitiesTasks);
SendUserFirstResultToThinkAbout(firstDoneTask.Result);
await Task.WhenAll(bestPathThroughCitiesTasks);
LetThemHaveItAll(bestPathThroughCitiesTasks.Select(t => t.Result)); The point of the above example is that it uses flexibility, expressiveness and ecosystem of TPL where use of own threads, while can certainly be done, would not be nearly as convenient and simple. Such code would require a mechanism where a thread notifies main code that result is ready, main code has to manage threads and listen for completions so as to know where to pick up first result, join thread back, then wait for the rest. Again, can be done, but far less convenient. And yet the above will run on separate threads doing long computations while perfectly protecting the thread pool and avoiding the problem described in the advice. I'd suggest amending the advice to make it clear when to use P.S. I think this confusion comes from people understanding "LongRunning" as wallclock time, rather than long contiguous block of operation that won't result in multiple subtasks reducing the computation a bunch of "ShortRunning" computations. If that understanding is there, then when to use |
Feel free to send a PR 😄 , I don't disagree. |
There's a note regarding avoidance of
TaskCreationOptions.LongRunning
:The code sample then proceeds to use:
The bit I'm missing is how we go from "API is awkward" to "look, you're better off with Thread.Start"; is it for reasons of cumbersome syntax or is it because it's outright better and/or
TaskCreationOptions.LongRunning
should all-but be deprecated that we go to the lengths of actually creating a thread explicitly? I propose to use the answer to eithera) make a PR to switch the example to use
Task.Factory.StartNew
, orb) clarify the note to explicitly indicate that
Task.Factory.StartNew
should be avoided, and(Thread {IsBackground = true}).Start()
is the recommended alternativeFor instance in
Serilog.Sinks.Async
, we create the thread usingTaskCreationOptions.LongRunning
The text was updated successfully, but these errors were encountered: