[issue24882] ThreadPoolExecutor doesn't reuse threads until #threads == max_workers

Antoine Pitrou added the comment: Thank you for your contribution iunknwn! -- resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker

[issue24882] ThreadPoolExecutor doesn't reuse threads until #threads == max_workers

Antoine Pitrou added the comment: New changeset 904e34d4e6b6007986dcc585d5c553ee8ae06f95 by Antoine Pitrou (Sean) in branch 'master': bpo-24882: Let ThreadPoolExecutor reuse idle threads before creating new thread (#6375)

[issue24882] ThreadPoolExecutor doesn't reuse threads until #threads == max_workers

Antoine Pitrou added the comment: Thomas, I think that's a good argument, so perhaps we should do this (strive to reuse threads) after all. -- ___ Python tracker ___

[issue24882] ThreadPoolExecutor doesn't reuse threads until #threads == max_workers

Change by Pierre Glaser : -- nosy: +pierreglaser ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue24882] ThreadPoolExecutor doesn't reuse threads until #threads == max_workers

Thomas added the comment: We ran into this issue in the context of asyncio which uses an internal ThreadPoolExecutor to provide an asynchronous getaddrinfo / getnameinfo. We observed an async application spawned more and more threads through several reconnects. With a maximum of 5 x CPUs

[issue24882] ThreadPoolExecutor doesn't reuse threads until #threads == max_workers

Brian Quinlan added the comment: After playing with it for a while, https://github.com/python/cpython/pull/6375 seems reasonable to me. It needs tests and some documentation. Antoine, are you still -1 because of the complexity increase? -- ___

[issue24882] ThreadPoolExecutor doesn't reuse threads until #threads == max_workers

Brian Quinlan added the comment: When I first wrote and started using ThreadPoolExecutor, I had a lot of code like this: with ThreadPoolExecutor(max_workers=500) as e: e.map(download, images) I didn't expect that `images` would be a large list but, if it was, I wanted all of the

[issue24882] ThreadPoolExecutor doesn't reuse threads until #threads == max_workers

iunknwn added the comment: I feel like there are two reasonable options here: 1) We can implement a thread pool with basic resource tracking. This means idle threads get recycled, and threads that have been sitting idle for a while are terminated as demand drops, so

[issue24882] ThreadPoolExecutor doesn't reuse threads until #threads == max_workers

INADA Naoki added the comment: Why not just remove TODO comment? Thread is cheap, but not zero-cost. -- nosy: +inada.naoki ___ Python tracker

[issue24882] ThreadPoolExecutor doesn't reuse threads until #threads == max_workers

iunknwn added the comment: Done - as recommend, I've opened a new PR that changes the behavior to spawn all worker threads when the executor is created. This eliminates all the thread logic from the submit function. -- ___ Python

[issue24882] ThreadPoolExecutor doesn't reuse threads until #threads == max_workers

Change by iunknwn : -- pull_requests: +6224 ___ Python tracker ___ ___ Python-bugs-list

[issue24882] ThreadPoolExecutor doesn't reuse threads until #threads == max_workers

Antoine Pitrou added the comment: Creating a new PR would be cleaner IMHO. -- ___ Python tracker ___

[issue24882] ThreadPoolExecutor doesn't reuse threads until #threads == max_workers

iunknwn added the comment: Alright - I'll put together another patch that removes the logic, and spins up all threads during initialization. Do you want me to create a completely new PR, or just update my existing one? -- ___

[issue24882] ThreadPoolExecutor doesn't reuse threads until #threads == max_workers

Antoine Pitrou added the comment: > That said, if threads are cheap, why not just create all the work threads on > initialization, and then remove all the logic entirely? That would sound reasonable to me. bquinlan has been absent for a long time, so I wouldn't expect an

[issue24882] ThreadPoolExecutor doesn't reuse threads until #threads == max_workers

iunknwn added the comment: The existing behavior seems strange (and isn't well documented). The code had a TODO comment from bquinlan to implement idle thread recycling, so that was why I made the change. That said, if threads are cheap, why not just create all the work

[issue24882] ThreadPoolExecutor doesn't reuse threads until #threads == max_workers

Antoine Pitrou added the comment: Side note: > One concern I do have - while writing the patch, I noticed the existing > submit method (specifically the adjust_thread_count function) isn't thread > safe. True. The executor is obviously thread-safe internally (as it handles

[issue24882] ThreadPoolExecutor doesn't reuse threads until #threads == max_workers

Antoine Pitrou added the comment: > If each worker thread ties up other resources in an application, such as > handles to server connections, conserving threads could have a significant > impact. You may want to implement a pooling mechanism for those connections,

[issue24882] ThreadPoolExecutor doesn't reuse threads until #threads == max_workers

Antoine Pitrou added the comment: I'm not fond of this proposal. The existing behaviour is harmless; especially for a thread pool, since threads are cheap resources. Improving the logic a bit might seem nice, but it also complicates the executor implementation a bit more.

[issue24882] ThreadPoolExecutor doesn't reuse threads until #threads == max_workers

Change by Ned Deily : -- nosy: +bquinlan, pitrou ___ Python tracker ___ ___

[issue24882] ThreadPoolExecutor doesn't reuse threads until #threads == max_workers

iunknwn added the comment: I've submitted a PR that should resolve this - it uses a simple atomic counter to ensure new threads are not created if existing threads are idle. One concern I do have - while writing the patch, I noticed the existing submit method (specifically

[issue24882] ThreadPoolExecutor doesn't reuse threads until #threads == max_workers

Change by iunknwn : -- nosy: +iunknwn ___ Python tracker ___ ___ Python-bugs-list mailing

[issue24882] ThreadPoolExecutor doesn't reuse threads until #threads == max_workers

Change by Roundup Robot : -- keywords: +patch pull_requests: +6087 stage: -> patch review ___ Python tracker ___

[issue24882] ThreadPoolExecutor doesn't reuse threads until #threads == max_workers

David MacKenzie added the comment: If each worker thread ties up other resources in an application, such as handles to server connections, conserving threads could have a significant impact. That's the situation for an application I am involved with. I've written and tested a patch to make

[issue24882] ThreadPoolExecutor doesn't reuse threads until #threads == max_workers

David MacKenzie added the comment: This issue seems to overlap with 14119. -- ___ Python tracker ___ ___

[issue24882] ThreadPoolExecutor doesn't reuse threads until #threads == max_workers

Josh Rosenberg added the comment: Is there a good reason to worry about overeager worker spawning? ProcessPoolExecutor spawns all workers when the first work item is submitted ( https://hg.python.org/cpython/file/3.4/Lib/concurrent/futures/process.py#l361 ), only ThreadPoolExecutor even makes

[issue24882] ThreadPoolExecutor doesn't reuse threads until #threads == max_workers

Torsten Landschoff added the comment: For demonstration purposes, here is a small example specifically for Linux which shows how each request starts a new thread even though the client blocks for each result. -- nosy: +torsten Added file:

[issue24882] ThreadPoolExecutor doesn't reuse threads until #threads == max_workers

Changes by Matt Spitz mattsp...@gmail.com: -- title: ThreadPoolExceutor doesn't reuse threads until #threads == max_workers - ThreadPoolExecutor doesn't reuse threads until #threads == max_workers ___ Python tracker rep...@bugs.python.org

[issue24882] ThreadPoolExecutor doesn't reuse threads until #threads == max_workers

Matt Spitz added the comment: On further investigation, it appears that we can't just check against the queue length, as it doesn't indicate whether threads are doing work or idle. A change here will need a counter/semaphore to keep track of the number of idle/working threads, which may have