[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: Re: [Panel-devel] dataengines and timing revisited
From: Michael Olbrich <michael-olbrich () web ! de>
Date: 2007-08-31 18:16:44
Message-ID: 20070831181644.GA24239 () c027 ! apm ! etc ! tu-bs ! de
[Download RAW message or body]
[Attachment #2 (multipart/signed)]
[Attachment #4 (multipart/mixed)]
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
["container-delete.diff" (text/x-diff)]
Index: dataengine.cpp
===================================================================
--- dataengine.cpp (revision 706949)
+++ dataengine.cpp (working copy)
@@ -154,6 +154,7 @@
return;
}
+ connect(visualization, SIGNAL(destroyed(QObject*)), s, \
SLOT(checkUsage(QObject*)), Qt::QueuedConnection); connect(s, \
SIGNAL(updated(QString,Plasma::DataEngine::Data)),
visualization, SLOT(updated(QString,Plasma::DataEngine::Data)));
QMetaObject::invokeMethod(visualization, "updated",
@@ -171,6 +172,8 @@
disconnect(s, SIGNAL(updated(QString,Plasma::DataEngine::Data)),
visualization, SLOT(updated(QString,Plasma::DataEngine::Data)));
+ QMetaObject::invokeMethod(s, "checkUsage", Qt::QueuedConnection,
+ Q_ARG(QObject*, 0));
}
void DataEngine::connectAllSources(QObject* visualization) const
@@ -200,6 +203,8 @@
return DataEngine::Data();
}
+ QMetaObject::invokeMethod(s, "checkUsage", Qt::QueuedConnection,
+ Q_ARG(QObject*, 0));
return s->data();
}
@@ -296,6 +301,7 @@
SourceDict::iterator it = d->sources.find(source);
if (it != d->sources.end()) {
emit sourceRemoved(it.key());
+ it.value()->deleteLater();
d->sources.erase(it);
}
}
Index: datacontainer.h
===================================================================
--- datacontainer.h (revision 706949)
+++ datacontainer.h (working copy)
@@ -92,9 +92,13 @@
**/
void unused(const QString& source);
- protected:
- void connectNotify(const char *signal);
- void disconnectNotify(const char *signal);
+ private Q_SLOTS:
+ /**
+ * Check if the DataContainer is still in use.
+ * If not the signal "unused" will be emitted.
+ * Warning: The DataContainer may be invalid after calling this function.
+ */
+ void checkUsage(QObject * obj);
private:
class Private;
Index: datacontainer.cpp
===================================================================
--- datacontainer.cpp (revision 706949)
+++ datacontainer.cpp (working copy)
@@ -33,7 +33,6 @@
{}
DataEngine::Data data;
- int connectCount;
bool dirty : 1;
};
@@ -79,6 +78,16 @@
d->dirty = true;
}
+void DataContainer::checkUsage(QObject * obj)
+{
+ // Don't use 'obj'. It is either NULL are already deleted.
+ Q_UNUSED(obj);
+ if (receivers(SIGNAL(updated(QString, Plasma::DataEngine::Data))) < 1) {
+ // DO NOT CALL ANYTHING AFTER THIS LINE AS IT MAY GET DELETED!
+ emit unused(objectName());
+ }
+}
+
void DataContainer::checkForUpdate()
{
if (d->dirty) {
@@ -87,27 +96,6 @@
}
}
-void DataContainer::connectNotify(const char *signal)
-{
- if (QLatin1String(signal) == \
QMetaObject::normalizedSignature(SIGNAL(updated(QString, \
Plasma::DataEngine::Data))).constData()) {
- ++d->connectCount;
- }
-}
-
-void DataContainer::disconnectNotify(const char *signal)
-{
- if (QLatin1String(signal) == \
QMetaObject::normalizedSignature(SIGNAL(updated(QString, \
Plasma::DataEngine::Data))).constData()) {
- if (d->connectCount > 0) {
- --d->connectCount;
- }
-
- if (d->connectCount < 1) {
- // DO NOT CALL ANYTHING AFTER THIS LINE AS IT MAY GET DELETED!
- emit unused(objectName());
- }
- }
-}
-
} // Plasma namespace
#include "datacontainer.moc"
["signature.asc" (application/pgp-signature)]
_______________________________________________
Panel-devel mailing list
Panel-devel@kde.org
https://mail.kde.org/mailman/listinfo/panel-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic