[Panel-devel] dataengines and timing revisited

[Aaron J. Seigo]

On Friday 31 August 2007, Michael Olbrich wrote:
> On Wed, Aug 29, 2007 at 08:40:58PM -0600, Aaron J. Seigo wrote:
> > 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

if the engine has an update frequency, yes. otherwise if there is no update 
frequency set in the engine (or it doesn't supply its own timer), then the 
applet will get one updated call only. otherwise, yes.

> (minus any skipped updates when the data didn't 
> change), right?

i'm not strictly handling "when the data doesn't change" yet. if the engine 
calls setData we trust it knows what it is doing and that the data has 
changed. so if we make "data didn't change" equivalent to "engine doesn't 
call setData" then yes =)

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

the possible call to update source happens in requestSource; otherwise it 
needs to wait for the next triggered update. connecting to a source shouldn't 
demand an update, as that really doesn't make any sense for many engines, 
particularly of type U(1) and for many C(2) situations. when the source gets 
updated in due time, an update of the applet will naturally happen.

> "possible call to updateSource" means the same check timerEvent()
> performs to prevent second updates.

well, of course, and since that check needs to be in updateSource to ensure 
it's always checked it's not a problem. =)

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

it should still listen disconnectNotify events: an object may disconnect 
without being deleted, and i don't trust that the only place that will ever 
happen is in disconnectSource.

having to use invokeMethod (to queue the call for later) shows how ugly this 
approach is, actually. there is the issue of items calling query() on a 
source that isn't connected to anything else (and so gets created and then 
just hangs out), but i'm really not very comfortable with the usage of 
invokeMethod there.

the rest of the patch looks fine

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

no, something came up yesterday that demanded my immediate attention.

> > [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... :-)

ah, that should've been referenced from "we round updateFrequency to a sane 
number"

-- 
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43

KDE core developer sponsored by Trolltech
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/panel-devel/attachments/20070831/65fa8868/attachment-0001.pgp