[issue20516] Concurrent.futures base concurrency improvement (with patch)

Antoine Pitrou added the comment: Woops. Thanks for noticing, Claudiu. I guess someone can take over the patch and finish the work, then :-) -- ___ Python tracker ___ __

[issue20516] Concurrent.futures base concurrency improvement (with patch)

Claudiu Popa added the comment: I'm not sure that Glenn receives this, he is not in the nosy list. 2014-07-18 16:45:36 glangford set nosy: - glangford -- nosy: +Claudiu.Popa ___ Python tracker __

[issue20516] Concurrent.futures base concurrency improvement (with patch)

Antoine Pitrou added the comment: Glenn, are you willing to update your patch to address the review comments? (I'm assuming you have been notified of them... if not, you can access the code review tool by clicking the "review" links near your patch) -- _

[issue20516] Concurrent.futures base concurrency improvement (with patch)

Changes by Glenn Langford : -- nosy: -glangford ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.p

[issue20516] Concurrent.futures base concurrency improvement (with patch)

Glenn Langford added the comment: Updated patch: Fixed bug where quick yield in as_completed() did not release lock on future. -- Added file: http://bugs.python.org/file33999/futures-base-v2.patch ___ Python tracker

[issue20516] Concurrent.futures base concurrency improvement (with patch)

Glenn Langford added the comment: > Actually, I think what you're seeing here is the context manager being garbage collected Yes indeed, I am with you now. Thanks for identifying the problem, and for your explanation. The future should not be yielded while locked. -- _

[issue20516] Concurrent.futures base concurrency improvement (with patch)

Charles-François Natali added the comment: > Note that the context manager will be called in this case to release the lock before f is yielded to the caller. > > class MiniContext(): > def __init__(self): > pass > > def __enter__(self): > print('Hello') > > def __exit_

[issue20516] Concurrent.futures base concurrency improvement (with patch)

Glenn Langford added the comment: > if the future is cancelled or finished, control is yielded back to the caller > with the condition's lock held. Hmmm...I don't agree. I assume you are looking at this code: with f._condition: # Lock the Future; yield if completed or add our Waiter

[issue20516] Concurrent.futures base concurrency improvement (with patch)

Charles-François Natali added the comment: Just a quick comment on the patch: in as_completed(), if the future is cancelled or finished, control is yielded back to the caller with the condition's lock held. As a general rule, libraries should not yield control to the caller with a lock held, b

[issue20516] Concurrent.futures base concurrency improvement (with patch)

Antoine Pitrou added the comment: Thanks for the patch. This will have to wait for 3.5, since 3.4 is currently in feature freeze stage. -- ___ Python tracker ___ ___

[issue20516] Concurrent.futures base concurrency improvement (with patch)

Changes by Antoine Pitrou : -- stage: -> patch review versions: -Python 3.4 ___ Python tracker ___ ___ Python-bugs-list mailing list

[issue20516] Concurrent.futures base concurrency improvement (with patch)

New submission from Glenn Langford: The current approach taken in as_completed() and wait() is to immediately lock the entire set of given Futures simultaneously. This limits concurrency, particularly when the number of futures is large, and also requires the complete set of Futures to be know