[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: Issues with Solid from trunk and qtcreator 2.0.1...
From:       Dawit A <adawit () kde ! org>
Date:       2010-11-02 20:09:10
Message-ID: AANLkTik67QCV9YJNJcyap-Yxjpy4AYrii++MSwSO_1KE () mail ! gmail ! com
[Download RAW message or body]

On Mon, Nov 1, 2010 at 2:28 PM, Lukáš Tinkl <ltinkl@redhat.com> wrote:
> Dne Po 1. listopadu 2010 18:25:33 Dawit A napsal(a):
>> On Mon, Nov 1, 2010 at 10:19 AM, Lukáš Tinkl <ltinkl@redhat.com> wrote:
>> > Dne Ne 31. října 2010 03:32:52 Dawit A napsal(a):
>> >> On Wed, Oct 27, 2010 at 12:24 PM, Dawit A <adawit@kde.org> wrote:
>> >> > On Wed, Oct 27, 2010 at 7:48 AM, Lukáš Tinkl <ltinkl@redhat.com> wrote:
>> >> >> Dne St 27. října 2010 00:57:27 Christophe Giboudeaux napsal(a):
>> >> >>> Hi,
>> >> >>>
>> >> >>> Le 27/10/2010 00:20, Dawit A a écrit :
>> >> >>> > Does anyone have a problem opening project files (CTRL+O) in
>> >> >>> > qtcreator 2.0.1 (Qt 4.7) with the current trunk version of kdelibs
>> >> >>> > ?
>> >> >>> >
>> >> >>> > For me the minute I try to open a project, the screen flashes as
>> >> >>> > if to paint the file dialog and then qtcreator completely
>> >> >>> > freezes. Using gdb (backtrace) I see that the issue seems to
>> >> >>> > occur when the file dialog attempts to find Places information
>> >> >>> > through Solid. Solid in turn queries udisks (no hal on my system)
>> >> >>> > through dbus to retrieve information about each of the several
>> >> >>> > devices available on my machine and takes a very very very long
>> >> >>> > time (many minutes) to do it. Any insight I can get into this
>> >> >>> > would be helpful before I waste even more time to try and find
>> >> >>> > out what could possibly be causing this issue for me...
>> >> >>>
>> >> >>> See https://bugs.kde.org/show_bug.cgi?id=253039
>> >> >>
>> >> >> Strangely enough it happens only in non-KDE aplications anb i can't
>> >> >> reproduce it... (not running HAL on my Fedora either); for me the
>> >> >> open dialog in eg. Qt Creator shows immediately.
>> >> >
>> >> > I am not running HAL on my system either. In fact hal is not even
>> >> > installed at all. And the freeze happens for non-KDE apps only as
>> >> > mentioned above. I was able to reproduce the problem with QtCreator
>> >> > and VLC so far. I have added a comment on the bug report given above
>> >> > with backtrace information from QtCreator...
>> >>
>> >> On my system commenting out the attempt to prefill the cache in
>> >> UDisksManager's ctor solves the problem. I really do not see the
>> >> beneift of doing that in the ctor anyhow...
>> >>
>> >> Regards,
>> >> Dawit A.
>> >
>> > You are probably right, this was made in an attempt to speed up things
>> > but it turned out the problem had been somewhere else (in the device
>> > properties cache). I will remove it, please retest as I am not getting
>> > those problems you mentioned.
>>
>> You reverted back your commit though... Did it break something ?
>> Anyhow, for me it solved the noticeble lags in KDE apps and the very
>> very long delays in QtCreator. However, the issue of the very very
>> long delays in VLC still persist even after removing that line...
>
> Well it essentially broke things, so I reverted it :) you can check with
> solid-hardware details <udi>

That is easy enough to address since it is caused by m_deviceCache not
being populated before it is use. BTW, the temporary result variable
in allDevices is completely unnecessary. Since it simply ends up being
assigned to m_deviceCache at the end, why not use m_deviceCache in the
first place and avoid unecessary copies ? Anyways, attached is a patch
that should fix the solid-hardware issue as well as cleanup the
allDevices function...

["solid_udisks_udisksmanager.patch" (application/octet-stream)]

Index: solid/backends/udisks/udisksmanager.cpp
===================================================================
--- solid/backends/udisks/udisksmanager.cpp	(revision 1192125)
+++ solid/backends/udisks/udisksmanager.cpp	(working copy)
@@ -56,8 +56,6 @@
                 this, SLOT(slotDeviceRemoved(QDBusObjectPath)));
         connect(&m_manager, SIGNAL(DeviceChanged(QDBusObjectPath)),
                 this, SLOT(slotDeviceChanged(QDBusObjectPath)));
-
-        m_deviceCache = allDevices(); // prefill the cache
     }
 }
 
@@ -89,9 +87,10 @@
 {
     QStringList result;
 
+    //qDebug() << Q_FUNC_INFO << "parentUdi:" << parentUdi << "type:" << type;
     if (!parentUdi.isEmpty())
     {
-        foreach (const QString & udi, m_deviceCache)
+        Q_FOREACH (const QString & udi, deviceCache())
         {
             UDisksDevice device(udi);
             if (device.queryDeviceInterface(type) && device.parentUdi() == \
parentUdi) @@ -102,7 +101,7 @@
     }
     else if (type != Solid::DeviceInterface::Unknown)
     {
-        foreach (const QString & udi, m_deviceCache)
+        Q_FOREACH (const QString & udi, deviceCache())
         {
             UDisksDevice device(udi);
             if (device.queryDeviceInterface(type))
@@ -111,20 +110,21 @@
 
         return result;
     }
-    else
-        return m_deviceCache;
+
+    return deviceCache();
 }
 
 QStringList UDisksManager::allDevices()
 {
+    //qDebug() << "*****" << Q_FUNC_INFO;
     m_knownDrivesWithMedia.clear();
+    m_deviceCache.clear();
 
-    QStringList result;
-    result << udiPrefix();
+    m_deviceCache << udiPrefix();
 
-    foreach(const QString & udi, allDevicesInternal())
+    Q_FOREACH(const QString & udi, allDevicesInternal())
     {
-        result.append(udi);
+        m_deviceCache.append(udi);
 
         UDisksDevice device(udi);
         if (device.queryDeviceInterface(Solid::DeviceInterface::OpticalDrive)) // \
forge a special (separate) device for optical discs @@ -133,14 +133,12 @@
             {
                 if (!m_knownDrivesWithMedia.contains(udi))
                     m_knownDrivesWithMedia.append(udi);
-                result.append(udi + ":media");
+                m_deviceCache.append(udi + ":media");
             }
         }
     }
 
-    m_deviceCache = result; // set the cache
-
-    return result;
+    return m_deviceCache;
 }
 
 QStringList UDisksManager::allDevicesInternal()
@@ -153,7 +151,7 @@
     }
 
     QStringList retList;
-    foreach (const QDBusObjectPath &path, reply.value()) {
+    Q_FOREACH(const QDBusObjectPath &path, reply.value()) {
         retList << path.path();
     }
 
@@ -217,4 +215,13 @@
     }
 }
 
+const QStringList& Solid::Backends::UDisks::UDisksManager::deviceCache()
+{
+    if (m_deviceCache.isEmpty())
+        allDevices();
+
+    return m_deviceCache;
+}
+
+
 #include "backends/udisks/udisksmanager.moc"
Index: solid/backends/udisks/udisksmanager.h
===================================================================
--- solid/backends/udisks/udisksmanager.h	(revision 1192125)
+++ solid/backends/udisks/udisksmanager.h	(working copy)
@@ -55,6 +55,7 @@
     void slotDeviceChanged(const QDBusObjectPath &opath);
 
 private:
+    const QStringList& deviceCache();
     QStringList allDevicesInternal();
     QStringList m_knownDrivesWithMedia;  // list of known optical drives which \
contain a media  QSet<Solid::DeviceInterface::Type> m_supportedInterfaces;



[prev in list] [next in list] [prev in thread] [next in thread]