[Panel-devel] dataengines and timing revisited

[Michael Olbrich]

Hi,

On Wed, Aug 29, 2007 at 08:40:58PM -0600, Aaron J. Seigo wrote:
> we can divide the problem into three parts:
> 
> Engine updating (U) its data
> 	U(1) it knows when to update its data, don't try and tell it otherwise
> 	U(2) engine updates on demand and should avoid updating otherwise
> 
> Engine data collection (C) methods
> 	C(1) synchronous
> 	C(2) asynchronous
> 
> Applet access (A) to data
> 	A(1) wants data whenever it arrives/changes (rely on the engine)
> 	A(2) wants data updated periodically (every N ms kick the engine)
[...]
> public:
> setMinimumUpdateFrequency(int) and int minimumUpdateFrequency() const
> 	sets the minimumt time in ms between updates.
> 		< 0 == never update on demand; just let the engine handle it
> 		0 == update immediately on (every) demand
> 		>0 == the number of ms that must pass before updates
> 	i'm still not sure on the default. maybe 500ms?
> 	an rss feed engine may want to have it at 60000 or even higher
> 	a cpu temp engine might want it to 0ms or perhaps 20ms
> 	a hotplug engine probably wants it at -1
> 
> protected:
> setUpdateFrequency(int) and int updateFrequency() const
> 	provides a built in timer for the engine (using timerEvent()) and cooperates
> 	with minUpdateFrequency(), so if an applet kicks an update for a source this
> 	won't cause a second update to that source

So in a combination of U(2) and A(1) the applet receives updates with
this update frequency (minus any skipped updates when the data didn't
change), right?

> connectSource and connectAllsources now take an optional update frequency, 
> much as in Michael's patch
> 
> the code then works something like this:
> 
> connectSource will connect the applet to the DataContainer's updated() signal 
> if the updateFrequency < 1. otherwise we round updateFrequency to a sane 
> number and create a QObject that stands in and emits the signal every 
> qMin(updateFrequency, minimumUpdateFrequency()).

This sounds wrong. I would expect a possible call to updateSource and an
update of the applet with the (sanitized) updateFrequency.
"possible call to updateSource" means the same check timerEvent()
performs to prevent second updates.

> care needs to be taken to ensure that:
[...]
>    - DataContainers are auto-removed appropriately

This does not work right now. The problem is that disconnectNotify() is
only called for expicit disconnects and not when the receiver is
deleted. Atached is a patch that uses a different method to detect
unused DataContainers.
It also adds the actual deletion of the DataContainer. Currently the
memory is leaking.

> this is obviously more complicated than what is there right now but i think it 
> is a bit more straight forward than Michael's original patch, introduces 
> fewer classes and doesn't introduce regressions in behaviour AFAICS. if there 
> are no objections to this approach, i will be finishing this up and 
> committing it tomorrow.

It's not commited yet is it? When I saw that line earlier today I wanted
to comment on the actual code but I couldn't find it.

> remaining caveats: the clock has an interesting problem where it really wants 
> to pin the ticks to clock ticks, especially for the minutes-only ... i have 
> yet to figure out how to arrange that. without it, it is possible for the 
> ticks to drift dramatically so that the time is up 59s out. maybe it's just 
> me, but that would suck =) if someone provides a clock that just shows the 
> hour, that could be even worse. then again, perhaps the clock, as a special 
> case, just continues to manage its own update ticks.

Well, you could subclass the object created in connectSource and handle
the drift there. It seems like overkill for one engine though.

> other cool things that could be added: sharing ticks between engines. feels 
> like a premature optimization at this point, though it might be nice for 
> power consumption ... in any case, we'll see.

Well, that would be easy to add after these changes. There should be no
timers in the individual applets any more.

> [2] nearest 50ms? that gives up to 20 updates a second. should be enough?

I think you missed the "[2]" in the body of the mail... :-)

michael

-------------- next part --------------
A non-text attachment was scrubbed...
Name: container-delete.diff
Type: text/x-diff
Size: 3514 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/panel-devel/attachments/20070831/271c8c92/attachment.bin 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://mail.kde.org/pipermail/panel-devel/attachments/20070831/271c8c92/attachment.pgp