homepage

This patch is against Python trunk, but it could be easily targeted for Python 3.2 . It is meant to extract the scheduler updates from issue1641 without mucking with asyncore. It's reach is reduced and simplified, which should make maintenance a bit easier.

This must be retargeted to 3.2. Also, the patch lacks some tests.
It seems PEP 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.

By the way, the patch lacks docs for new public APIs.

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.

Created issue 8687 to address the test suite problem.

sched.py tests have been checked in.

Josiah are you still interested in this?

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.

Updated patch adding a synchronized argument to scheduler class and updating doc is in attachment.

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.

Are you suggesting to enable thread-synchronization by default and get rid of explicit "synchronized" argument?

> 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 no
significant performance degradation.

This is what I get by using bench.py script attached to issue13451:
CURRENT VERSION (NO LOCK)
test_cancel : time=0.67457 : calls=1 : stdev=0.00000
test_empty : time=0.00025 : calls=1 : stdev=0.00000
test_enter : time=0.00302 : calls=1 : stdev=0.00000
test_queue : time=6.31787 : calls=1 : stdev=0.00000
test_run : time=0.00741 : calls=1 : stdev=0.00000
LOCK WITH DECORATOR (no synchronization)
test_cancel : time=0.70455 : calls=1 : stdev=0.00000
test_empty : time=0.00050 : calls=1 : stdev=0.00000
test_enter : time=0.00405 : calls=1 : stdev=0.00000
test_queue : time=6.23341 : calls=1 : stdev=0.00000
test_run : time=0.00776 : calls=1 : stdev=0.00000
LOCK WITHOUT DECORATOR (always synchronized)
test_cancel : time=0.69625 : calls=1 : stdev=0.00000
test_empty : time=0.00053 : calls=1 : stdev=0.00000
test_enter : time=0.00397 : calls=1 : stdev=0.00000
test_queue : time=6.36999 : calls=1 : stdev=0.00000
test_run : time=0.00783 : calls=1 : stdev=0.00000
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.
Maybe it makes sense to provide it as a separate class (SynchronizedScheduler).

> 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.

New patch in attachment. I'll commit it later today.

New changeset f5aed0dba844 by Giampaolo Rodola' in branch 'default':
Fix #8684: make sched.scheduler class thread-safe
http://hg.python.org/cpython/rev/f5aed0dba844 

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

New changeset 50267d2bb320 by Giampaolo Rodola' in branch 'default':
(bug #8684) fix 'fedora without thread buildbot' as per http://bugs.python.org/issue8684
http://hg.python.org/cpython/rev/50267d2bb320 

This should now be fixed. Thanks for signaling.