
This issue trackerhas been migrated toGitHub, and is currentlyread-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.
Created on2010-05-11 04:27 byjosiahcarlson, last changed2022-04-11 14:57 byadmin. This issue is nowclosed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| sched.patch | josiahcarlson,2010-05-11 04:27 | some improvements to the scheduler library | ||
| sched-thread-safe.patch | giampaolo.rodola,2011-12-10 21:10 | review | ||
| sched-thread-safe.patch | giampaolo.rodola,2011-12-12 11:41 | review | ||
| Messages (19) | |||
|---|---|---|---|
| msg105483 -(view) | Author: Josiah Carlson (josiahcarlson)*![]() | Date: 2010-05-11 04:27 | |
This patch is against Python trunk, but it could be easily targeted for Python 3.2 . It is meant to extract the scheduler updates fromissue1641 without mucking with asyncore. It's reach is reduced and simplified, which should make maintenance a bit easier. | |||
| msg105497 -(view) | Author: Antoine Pitrou (pitrou)*![]() | Date: 2010-05-11 11:12 | |
This must be retargeted to 3.2. Also, the patch lacks some tests.It seemsPEP 8 compliance could be better: function names shouldn't be CamelCased.Is LocalSynchronize() an implementation detail rather than a public API? If so, I think it would be better if it started with an underscore. | |||
| msg105499 -(view) | Author: Antoine Pitrou (pitrou)*![]() | Date: 2010-05-11 11:15 | |
By the way, the patch lacks docs for new public APIs. | |||
| msg105502 -(view) | Author: Giampaolo Rodola' (giampaolo.rodola)*![]() | Date: 2010-05-11 12:21 | |
What are the enhancements introduced by this patch?A faster cancel() implementation?If so some simple benchmarks showing the speed improvement of cancel() and eventually also other methods like enterabs() and run() would be nice to have.I've noticed there are no existing tests for the sched module.This is very bad. I think the first thing to do is adding tests, make sure they pass with the current implementation, then apply the patch and make sure they keep passing.Obviously it is also crucial that this patch is fully backward compatible with the current implementation. | |||
| msg105504 -(view) | Author: Giampaolo Rodola' (giampaolo.rodola)*![]() | Date: 2010-05-11 12:31 | |
Createdissue 8687 to address the test suite problem. | |||
| msg112778 -(view) | Author: Giampaolo Rodola' (giampaolo.rodola)*![]() | Date: 2010-08-04 09:32 | |
sched.py tests have been checked in. | |||
| msg115697 -(view) | Author: Giampaolo Rodola' (giampaolo.rodola)*![]() | Date: 2010-09-06 11:03 | |
Josiah are you still interested in this? | |||
| msg148409 -(view) | Author: Giampaolo Rodola' (giampaolo.rodola)*![]() | Date: 2011-11-26 14:40 | |
Looking back at this patch, I think we can extract the thread-synchronization parts and the peek() method, as they're both valuable additions, especially the first one.The very sched doc says:> In multi-threaded environments, the scheduler class has limitations > with respect to thread-safety, inability to insert a new task before > the one currently pending in a running scheduler, and holding up the > main thread until the event queue is empty. Instead, the preferred > approach is to use the threading.Timer class instead. | |||
| msg149190 -(view) | Author: Giampaolo Rodola' (giampaolo.rodola)*![]() | Date: 2011-12-10 21:10 | |
Updated patch adding a synchronized argument to scheduler class and updating doc is in attachment. | |||
| msg149281 -(view) | Author: Antoine Pitrou (pitrou)*![]() | Date: 2011-12-12 09:52 | |
I'm not convinced by the decorator approach. Why not simply add "with self.lock" at the beginning of each protected method? It would actually save a function call indirection. | |||
| msg149283 -(view) | Author: Giampaolo Rodola' (giampaolo.rodola)*![]() | Date: 2011-12-12 10:09 | |
Are you suggesting to enable thread-synchronization by default and get rid of explicit "synchronized" argument? | |||
| msg149284 -(view) | Author: Antoine Pitrou (pitrou)*![]() | Date: 2011-12-12 10:15 | |
> Are you suggesting to enable thread-synchronization by default and get> rid of explicit "synchronized" argument?That would be even better indeed, if you find out that there's nosignificant performance degradation. | |||
| msg149287 -(view) | Author: Giampaolo Rodola' (giampaolo.rodola)*![]() | Date: 2011-12-12 11:13 | |
This is what I get by using bench.py script attached toissue13451:CURRENT VERSION (NO LOCK)test_cancel : time=0.67457 : calls=1 : stdev=0.00000test_empty : time=0.00025 : calls=1 : stdev=0.00000test_enter : time=0.00302 : calls=1 : stdev=0.00000test_queue : time=6.31787 : calls=1 : stdev=0.00000test_run : time=0.00741 : calls=1 : stdev=0.00000LOCK WITH DECORATOR (no synchronization)test_cancel : time=0.70455 : calls=1 : stdev=0.00000test_empty : time=0.00050 : calls=1 : stdev=0.00000test_enter : time=0.00405 : calls=1 : stdev=0.00000test_queue : time=6.23341 : calls=1 : stdev=0.00000test_run : time=0.00776 : calls=1 : stdev=0.00000LOCK WITHOUT DECORATOR (always synchronized)test_cancel : time=0.69625 : calls=1 : stdev=0.00000test_empty : time=0.00053 : calls=1 : stdev=0.00000test_enter : time=0.00397 : calls=1 : stdev=0.00000test_queue : time=6.36999 : calls=1 : stdev=0.00000test_run : time=0.00783 : calls=1 : stdev=0.00000Versions #2 and #3 have the same cost, so it's better to get rid of the explicit argument and always use the lock (version #3).Differences between #1 and #3 suggest that introducing the lock obviously have a cost though.It's not too high IMO, but I couldn't say whether it's acceptable or not.Maybe it makes sense to provide it as a separate class (SynchronizedScheduler). | |||
| msg149288 -(view) | Author: Antoine Pitrou (pitrou)*![]() | Date: 2011-12-12 11:17 | |
> Versions #2 and #3 have the same cost, so it's better to get rid of> the explicit argument and always use the lock (version #3).> Differences between #1 and #3 suggest that introducing the lock> obviously have a cost though.> It's not too high IMO, but I couldn't say whether it's acceptable or> not.It looks quite negligible to me. Nobody should be affected in practice. | |||
| msg149290 -(view) | Author: Giampaolo Rodola' (giampaolo.rodola)*![]() | Date: 2011-12-12 11:41 | |
New patch in attachment. I'll commit it later today. | |||
| msg149442 -(view) | Author: Roundup Robot (python-dev)![]() | Date: 2011-12-14 12:34 | |
New changesetf5aed0dba844 by Giampaolo Rodola' in branch 'default':Fix#8684: make sched.scheduler class thread-safehttp://hg.python.org/cpython/rev/f5aed0dba844 | |||
| msg149741 -(view) | Author: Charles-François Natali (neologix)*![]() | Date: 2011-12-18 09:52 | |
This broken the "Fedora without thread buildbot", since sched now requires the threading module:http://www.python.org/dev/buildbot/all/builders/AMD64 Fedora without threads 3.x/builds/1250/steps/test/logs/stdio | |||
| msg149883 -(view) | Author: Roundup Robot (python-dev)![]() | Date: 2011-12-19 18:12 | |
New changeset50267d2bb320 by Giampaolo Rodola' in branch 'default':(bug#8684) fix 'fedora without thread buildbot' as perhttp://bugs.python.org/issue8684http://hg.python.org/cpython/rev/50267d2bb320 | |||
| msg149884 -(view) | Author: Giampaolo Rodola' (giampaolo.rodola)*![]() | Date: 2011-12-19 18:13 | |
This should now be fixed. Thanks for signaling. | |||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:57:00 | admin | set | github: 52930 |
| 2011-12-19 18:13:26 | giampaolo.rodola | set | status: open -> closed messages: +msg149884 |
| 2011-12-19 18:12:08 | python-dev | set | messages: +msg149883 |
| 2011-12-18 09:52:05 | neologix | set | status: closed -> open nosy: +neologix messages: +msg149741 |
| 2011-12-14 12:36:09 | giampaolo.rodola | set | status: open -> closed resolution: fixed stage: patch review -> resolved |
| 2011-12-14 12:34:41 | python-dev | set | nosy: +python-dev messages: +msg149442 |
| 2011-12-12 11:41:05 | giampaolo.rodola | set | files: +sched-thread-safe.patch nosy: +rhettinger messages: +msg149290 |
| 2011-12-12 11:17:16 | pitrou | set | messages: +msg149288 |
| 2011-12-12 11:13:32 | giampaolo.rodola | set | messages: +msg149287 |
| 2011-12-12 10:15:47 | pitrou | set | messages: +msg149284 |
| 2011-12-12 10:09:24 | giampaolo.rodola | set | messages: +msg149283 |
| 2011-12-12 09:52:18 | pitrou | set | stage: patch review messages: +msg149281 versions: + Python 3.3, - Python 3.2 |
| 2011-12-10 21:10:10 | giampaolo.rodola | set | files: +sched-thread-safe.patch messages: +msg149190 |
| 2011-11-26 14:40:12 | giampaolo.rodola | set | messages: +msg148409 |
| 2011-01-07 14:49:41 | mark.dickinson | set | nosy: +mark.dickinson |
| 2010-09-06 11:03:12 | giampaolo.rodola | set | keywords:patch,patch,needs review messages: +msg115697 |
| 2010-08-04 09:32:28 | giampaolo.rodola | set | keywords:patch,patch,needs review messages: +msg112778 |
| 2010-05-11 13:07:31 | r.david.murray | set | keywords:patch,patch,needs review dependencies: +sched.py module doesn't have a test suite superseder:sched.py module doesn't have a test suite -> |
| 2010-05-11 12:31:52 | giampaolo.rodola | set | keywords:patch,patch,needs review superseder:sched.py module doesn't have a test suite messages: +msg105504 |
| 2010-05-11 12:21:52 | giampaolo.rodola | set | keywords:patch,patch,needs review messages: +msg105502 |
| 2010-05-11 11:15:18 | pitrou | set | keywords:patch,patch,needs review messages: +msg105499 |
| 2010-05-11 11:12:02 | pitrou | set | versions: + Python 3.2 nosy: +pitrou messages: +msg105497 keywords:patch,patch,needs review |
| 2010-05-11 04:27:48 | josiahcarlson | create | |