Skip to content
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

Update documentation from "Concurrent tasks" to "Maximum concurrent tasks" #19

Closed
Avasam opened this issue Feb 6, 2022 · 3 comments
Closed
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@Avasam
Copy link
Contributor

Avasam commented Feb 6, 2022

  1. There's a typo in the readme: "Cocurrent Tasks"
  2. It should be "Maximum Concurrent Tasks". As it may actually use less (see documentation for MaxDegreeOfParallelism https://docs.microsoft.com/en-us/dotnet/api/system.threading.tasks.paralleloptions.maxdegreeofparallelism?view=net-6.0 ). Setting it anything higher than the number of logical processors should have no effect as far as I understand. Tests on my machine seems to concur.
  3. Indicate in the documentation that setting it higher than the number of logical processors does nothing (or at least doesn't help). Having to specify the maximum is a very edge case. And anything less is only useful if you'd like to keep using your computer for something else while Unload is converting frames (or if your CPU weirdly has issues with using all cores at once)

Ref for logical processors:
image

(this next one I find optional, especially with the 3 points above and #17)
4. Default "Maximum concurrent tasks" to "Auto" or "Max". (which would not specify a MaxDegreeOfParallelism to ParallelOptions). Unless this is one of those edge cases When the thread pool's heuristics is unable to determine the right number of threads to use and could end up injecting too many threads.

@Avasam Avasam changed the title Update documentation for "Concurrent Tasks" Update documentation from "Concurrent tasks" to "Maximum concurrent tasks" Feb 6, 2022
@milankarman
Copy link
Owner

Definitely worth testing/updating. I'll admit that eyeballed the value on this as I was trying a lot of things to optimize so it's worth revisiting.

@milankarman milankarman added this to To do in Unload 1.3.0 via automation Feb 7, 2022
@milankarman milankarman added documentation Improvements or additions to documentation enhancement New feature or request labels Feb 7, 2022
@milankarman milankarman moved this from To do to In progress in Unload 1.3.0 Feb 16, 2022
@milankarman
Copy link
Owner

Played around with it for a little bit and got pretty much the same results as you did. I think I'd rather just remove the setting completely to keep it simple. Lowering it would really be a niche edge case and I think it would just be more confusing to keep it around.

@milankarman milankarman moved this from In progress to Done in Unload 1.3.0 Feb 17, 2022
@milankarman
Copy link
Owner

Closing this because the concurrent task setting is removed in version 1.3.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
No open projects
Unload 1.3.0
Ready for release
Development

No branches or pull requests

2 participants