-
Notifications
You must be signed in to change notification settings - Fork 180
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
New dotnet libraries and mechanisms #24
Comments
Hi Yoav, Can you please give examples of how you wish to use these features with STP? Regards, |
Hi Ami, The idea to combine it with ValueTask is just for having a better allocation than just using a regular Task. Regarding the 'System.Threading.Channels', one thing I like is the 'bounded queue' backpressure mechanism that automatically waits the queue will have a room for additional items when the queue is full (instead of throwing a 'queue is full' exception). Yoav |
Hi @yoav-melamed, By async/await I thought you meant that when you enqueue an async method it will continue to work on the STP. For example let's take this async method:
Today if you enqueue it into to the STP, the work item will complete after "first thing" is done, and "second thing" will be done on the .net Thread Pool (out of the STP threads). With an async/await feature the STP can do the second thing on the STP and only then will the work item complete. Is this a required feature? Or you just need the equeue and WaitForIdle to be awaitable ? Ami |
Hi @amibar In addition, since the WaitForIdle is an expensive blocking method, having it async would be great. |
Oh yeah, async/await feature so that you can do |
Hi @yoav-melamed & @MichelZ, I started to work on the async/await a while ago and I am still figuring out how to solve some scenarios. Regards, |
Hi @amibar
That is what I had in mind |
Hi @yoav-melamed, Thanks. The threads of the STP suppose to be worker threads, hence they should use the CPU. One of the ideas behind STP is to limit concurrency. As for WaitForIdleAsync() and GetResultAsync(), it's quite reasonable tom implement. What do you think? Regards, |
I often use it to limit concurrency, yes. |
Did you mean .ConfigureAwait(false) ? |
Yeah, that one :) |
Do you use the WorkItemGroups to limit concurrency or use the STP directly ? |
Both. We use STP in a few projects |
I have dilemas regarding async methods and the WorkItemGroup.
|
Hi Ami,
Same as @MichelZ , I'm using STP in several projects and it depends on the project I'm working on. Mainly I'm using the STP directly, but in some project, I need to have 4 workers that collect data each from a different source, and add it to a centralized queue to be processed (and each worker have a different priority) so - I'm using the WIG there
I thought a lot about what you said, and you are right - executing an async task will make the dotnet ThreadPool control the thread life cycle, which is the opposite of what we want. A Sample I wrote to demonstrate what I mean:
|
Hi @yoav-melamed, I looked at your sample and I realized that using the QueueWorkItem for a Task is not the best solution. When this project started (.net 1.1) the Task type didn’t exist yet, so I had to wrap delegates with objects (WorkItem) and return promises (IWorkItemResult). A Task is a promise, so I can harness its methods, instead of treating it like a delegate. So for a Task I prefer using RunTask, similar to Task.Run(), rewriting your sample it should look like this:
The RunTask also supports cancelletion token and priority. I can still implement the QueueWorkItem for tasks, but I don't think it necessary. What’s your opinion? I pushed a branch named async with what I did so far. Ami |
Hi @amibar Does the If so, would you be able to queue tasks also on WokItemGroups? or for now - only on the stp itself? |
Hi @amibar |
Ignore this comment... I saw the code:
You queue the task, convert it to WorkItem, and return a Promise (as you explain in your previous comment). |
Hi @yoav-melamed, The concurrency is of the threads not the tasks. Ami |
Hi @amibar |
Hi @yoav-melamed, I pushed anothee commit to the async branch. This time a WorkItemsGroup cannot have more than the Concurrency work items executing. Please check if it works for you. Thanks, |
Hi @amibar |
@yoav-melamed, check the WorkItemsGroup, I didn't update the STP itself. |
Hi @amibar
Now the execution is running as expected (if concurrency is 1, the items are being executed one after the other, if it's 5 (for example) so only 5 in parallel are invoked). The issue I noticed now is that since the |
Check https://stackoverflow.com/questions/271440/captured-variable-in-a-loop-in-c-sharp to solve this. You should copy i to another local variable before passing it to the Executor
|
Hi @amibar |
Hello Ami,
A chance to thank you for your great job!
I'm using STP a lot and this is defiantly my favorite library!
Do you consider for future releases some of the new dotnet libraries and mechanisms?
Thanks!
The text was updated successfully, but these errors were encountered: