[Milhouse]

I've been looking at a few "service" type addons, and as far as I can tell in order for xbmc.Monitor to function properly, and for it's notifications to be "consumed", xbmc.sleep() needs to be called at some point.

And in order for xbmc.abortRequested to be actioned in a timely manner, xbmc.sleep() needs to be called fairly frequently.

So, what you end up with is this:

Code:

class MyMonitorClass(xbmc.Monitor):

    def __init__( self, *args, **kwargs ):
        xbmc.Monitor.__init__(self)

    def onNotification(self, sender, method, data):
        ...

    def onScreensaverActivated(self):
        ...

    def onAbortRequested(self):
        ...

monitor = MyMonitorClass()

while not xbmc.abortRequested:
    xbmc.sleep(100) # process event notifications, check if aborted 10 times a second

which on a low powered device is wasteful as the thread is waking up 10 times a second and spinning around doing pretty much nothing.

Without the call to xbmc.sleep(), no xbmc.Monitor notification events are consumed. With a long xbmc.sleep(), events are consumed correctly, but now xbmc.abortRequested is not actioned fast enough during a shutdown.

Ideally what is needed is a very long (or even infinite) sleep() that can be interrupted whenever an abort occurs:

Code:

while not xbmc.abortRequested:
    xbmc.sleep(-1) # sleep forever, or until interrupted

Could the xbmc object be made to terminate the call to sleep() whenever an abort occurs?

Maybe a new method, xbmc.wake(), could be used to terminate the current threads call to xbmc.sleep(), with xbmc.wake() being called in xbmc.Monitor::onAbortRequested(). Or for that matter, it could be called by any of the other xbmc.Monitor event handlers.

This would be much more efficient than continuously waking up many times a second to check if anything needs to be done.

[Milhouse]

No thoughts on this (not even that I might be talking nonsense?)

[takoi]

The stuff about calling sleep being wasteful is nonsense, yes. I think what you're really asking for is a better abortRequested, and I agree. For instance having a function like

Code:

xbmc.waitForAbort()

that blocks until xbmc want to shutdown is a much nicer way of writing the so common while not xbmc.abortRequested loop. As for onAbortRequested I don't even understand the purpose of this because you can't use it to terminate the main thread in any way. You still have to check abortRequested.

[Milhouse]

Well, not waking up 10 times a second - ie. waking less often - does seem to result in reduced CPU load on a device like the R-Pi so I think there is some sense here (think about all the context switches etc., then multiply by all the scripts doing the same thing). The unfortunate side effect of this is that when it comes to shutting down XBMC will then choke on the unresponsive script.

But at the very least a better abort mechanism would seem like a valid request, thanks for confirming that.

[jfcarroll]

You are correct about the way it works. takoi's suggestion would be the easiest to implement.

Alternately for now, why not just poll every second? Is .1 second resolution really necessary?

[Milhouse]

Alternately for now, why not just poll every second? Is .1 second resolution really necessary?

No it's absolutely not, but this is what some addons are already doing.

As things stand it's a matter of finding the balance between efficiency (not waking too often) and still meeting the shutdown deadline which is about 5 seconds, so in theory it should be possible to wake every 1 second. However it's all rather hack-ish when having a mechanism that doesn't require periodic waking at all (and still responding quickly to a shutdown) would be the ideal.

xbmc.waitForAbort() sounds good, but it might still be worth adding a timeout property (eg. xbmc.waitForAbort(86400)) which would allow a service to do something only periodically (eg. once a minute, hour or even day) while not waking frequently AND yet still aborting promptly. It could just check the xbmc.abortRequested property to determine if the timer expired or if it's an actual abort.

[takoi]

Alternately for now, why not just poll every second? Is .1 second resolution really necessary?

Then you only have 4 seconds to cleanup. In worst case.

xbmc.waitForAbort() sounds good, but it might still be worth adding a timeout property (eg. xbmc.waitForAbort(86400)) which would allow a service to do something only periodically (eg. once a minute, hour or even day) while not waking frequently AND yet still aborting promptly. It could just check the xbmc.abortRequested property to determine if the timer expired or if it's an actual abort.

There's a standard way of doing that: let xbmc.waitForAbort(timeout) return true if aborted, false if it times out.

[Milhouse]

There's a standard way of doing that: let xbmc.waitForAbort(timeout) return true if aborted, false if it times out.

Yep, that would work. Doh.

[Milhouse]

@jfcarroll: would a trac ticket help this along?

[Martijn]

You are correct about the way it works. takoi's suggestion would be the easiest to implement.

Alternately for now, why not just poll every second? Is .1 second resolution really necessary?

why not also break the sleep if AbortRequested?
could that be integrated in the xbmc.sleep() code?

[Milhouse]

That was the original request, maybe changing its behaviour would cause compatibility issues? Although I'm not sure what issues there would be, as you would always want to exit the sleep on an abort...

[Martijn]

That was the original request, maybe changing its behaviour would cause compatibility issues? Although I'm not sure what issues there would be, as you would always want to exit the sleep on an abort...

i can't think of possible issues. it just jumps out of sleep earlier than some one would expect and XBMC kills the script and closes down.

Adding the other option as well seems a good idea.

[robweber]

Agreed. I've written a number of service related addons and always thought waking up only to check the xbmc.abortRequested condition was a waste of CPU. If you could tell xbmc.sleep() to really sleep the length you had intended knowing that xbmc would break the sleep condition if a shutdown was requested that would be great. You could sleep for 10 minutes for all the app cared since it would still be able to shutdown quickly and not wait for a poorly coded service that may be hanging up the system.

For addons that function strictly on the Monitor events xbmc.waitForAbort() would be nice. This would let your monitors function and basically let the service lay dormant until the Monitor function triggered an event to process.

[Milhouse]

Although xbmc.waitForAbort() may be the simpler solution, it would be a whole lot more useful if it could also support an optional timeout value at which point it then becomes just an interruptible xbmc.sleep() function, which is where this started.

Assuming interruptible xbmc.sleep() is actually possible (hopefully it is) then I'm not sure we need both xbmc.waitForAbort() and interruptible xbmc.sleep() as just the latter would suffice. Pass a value of None (which could even become the default) in order to request an indefinite timeout.

Edit: By the way, if the plan were to implement both interruptible xbmc.sleep() and xbmc.waitForAbort() I'm not going to complain, just pointing out the apparent overlap in functionality. I've also opened trac 15267 if this is required by anyone good enough to work on this.

[Milhouse]

I've done some more testing on a 1GHz Pi using top to monitor the CPU load of the individual LanguageInvoker threads.

For different sleep intervals when using the typical "monitor" pattern:

Code:

while not xbmc.abortRequested:
    xbmc.sleep(n)

it seems that for a value of n=1 the CPU load is (not surprisingly) high at 13% for the corresponding "LanguageInvoker" thread.

When n=10, the CPU load is 2.3%.

When n=100, the CPU load is a consistent 0.3%

When n=1000 or more, the CPU load is basically 0%, occasionally 0.3%, depending on how frequently top is polling.

Looking at the CPU usage of the various long-lived LanguageInvoker threads on my system, it seems that 4 out of the 9 LanguageInvoker threads are consistently using 0.3% CPU per thread, so my suspicion is these four threads are sleeping with 100ms intervals, waking up, doing nothing, then sleeping again.

An interval of 1000ms (or longer) would seem the optimal sleep interval but does of course slow down the abort sequence.

And while 0.3% CPU per thread isn't a great deal, when there are multiple LanguageInvoker threads all using 0.3% CPU its an amount that adds up on low powered devices. Sure, even 1-2% of wasted CPU isn't a great deal, but it's 1-2% (or more, depending on the number of addons) that doesn't need to be wasted if there existed a "better" xbmc.sleep() mechanism. Of course, getting developers to adopt the new, improved sleep mechanism is likely to be an uphill struggle assuming addon code changes are necessary.

The reason I've been looking into this is because I wanted to write yet another addon that would end up sleeping most of the time, but I've decided against writing it rather than add to the ever growing list of addons that waste CPU time while "sleeping".