[KPhotoAlbum] Import crash

Robert Krawitz rlk at alum.mit.edu
Thu May 18 02:59:56 BST 2017 

On Wed, 17 May 2017 21:07:25 +0200, Johannes Zarl-Zierl wrote:
> On Dienstag, 16. Mai 2017 21:54:01 CEST Robert Krawitz wrote:
>> On Mon, 15 May 2017 22:46:55 +0200, Johannes Zarl-Zierl wrote:
>> > 
>> > So I guess many of the imported images are already in the database? This
>> > would lead to a direct recursion. As a quick fix I guess we could bypass
>> > the direct call to aCopyJobCompleted(0) in copyNextFromExternal() in the
>> > default case...
>> I did it by having aCopyJobCompleted() set (clear) a flag to indicate
>> whether to continue looping.  But there's a new problem: as the import
>> progresses, I get messages like this:
>> 
>> kf5.kio.core: Invalid URL: QUrl("img_6004_c2.jpg")
>> kf5.kio.core: Invalid URL: QUrl("img_6005-5.jpg")
>> kf5.kio.core: Invalid URL: QUrl("img_6006-5.jpg")
>> kf5.kio.core: Invalid URL: QUrl("img_6007-6.jpg")
>> kf5.kio.core: Invalid URL: QUrl("img_6008-5.jpg")
>> kf5.kio.core: Invalid URL: QUrl("img_6009-6.jpg")
>> 
>> That's because the export file creates unique filenames even if the
>> files live in different directories (I presume because if you actually
>> exported the images, rather than the file, they'd live in one
>> directory so would need unique filenames):
>
> [snip]
>
>> So this probably isn't going to do what I want after all.  Rats.
>
> Just to be clear about the problem: You end up with a bunch of non-existing images in the database, in addition to the original that is already in the database?

I didn't let it run that far.  It was prompting me for the location of
each file, and I punted at that point.

> If so, maybe you can help things along by using the "Find duplicates" dialog.
>
> A proper fix needs to change the way we create export files. In the
> past, I was always hesitant to touch the .kim file format in order
> to avoid compatibility problems. Maybe it's time to revisit this
> decision...

I agree it needs to be backward compatible, so a newer kpa can import
an older .kim file.  I don't think the other direction is necessary;
if an old kpa can't import a new import file, you upgrade.

>> >> I might also note that during a (much smaller) import the
>> >> timeline bar constantly flashes, seemingly as each image is
>> >> added to the database; this resulted in my window manager
>> >> crashing.
>> > 
>> > Interesting. Report a bug with the window manager ;-)
>> > Honestly though, I guess we should do that more efficiently. It seems that
>> > ImportHandler::addNewRecord() calls DB::addImages for each image instead
>> > of assembling a list and adding the whole list at once...
>> I was looking at that, but I don't know whether it's safe to call
>> updateCategories() before addImages().  If not, this will have to be
>> done in two passes.
>
> Just looking at the code I would think that it is safe to change
> updateCategories() so that it accepts an ImageInfoList and call both
> addImages() and updateCategories() once after assembling the image
> list.

I'll put that on my list.

>> >> Finally, the process of comparing MD5 checksums against the database
>> >> appears to be extremely slow; it took hours to go through 90,000
>> >> images.  Perhaps the existing MD5 checksums need to be stored in a
>> >> hash, rather than a sequential comparison or something?
>> > 
>> > Looking at DB::MD5Map, this is already a map. I'd have to look more
>> > closely to see what's eating cpu in there...
>
>> One of the problems is with displaying the images page.
>> createImagesPage() is fast enough, but when it Qt then goes to display
>> it, it's deathly slow if there are a lot of images (presumably there's
>> some inefficiency in Qt with creating a dialog with thousands of
>> items in it -- or the way KPA is doing it, since file browsers have to
>> be able to display lots of items).  Even if I have it not try to
>> display a pixmap and/or label, it's very slow.
>
> I would assume that file managers don't need run into this issue
> because most file systems perform poorly when you stash thousands of
> files into a single directory.  For widgets that really have a huge
> number of items, I guess you could do progessive loading? I haven't
> looked into this, though...

I tried creating 100,000 files in one directory (touch $(seq 1
100000)).  I opened it with Dolphin; the browser loaded in something
under 1 second.

I suspect QLayout isn't really designed for that many items; it's very
flexible and tries hard to achieve the best layout for the widgets you
put into it.  If I run it under gdb, and break into it, it looks like
it's spending a huge amount of time doing stuff like this (all inside
that ImportExport::ImportDialog::exec() call):

(gdb) where
#0  0x00007ffff284768f in QWidgetPrivate::subtractOpaqueSiblings(QRegion&, bool*, bool) const () at /usr/lib64/libQt5Widgets.so.5
#1  0x00007ffff2821312 in  () at /usr/lib64/libQt5Widgets.so.5
#2  0x00007ffff2822e99 in  () at /usr/lib64/libQt5Widgets.so.5
#3  0x00007ffff2841f1f in QWidgetPrivate::syncBackingStore() ()
    at /usr/lib64/libQt5Widgets.so.5
#4  0x00007ffff2856a0d in QWidget::event(QEvent*) ()
    at /usr/lib64/libQt5Widgets.so.5
#5  0x00007ffff28131bc in QApplicationPrivate::notify_helper(QObject*, QEvent*) () at /usr/lib64/libQt5Widgets.so.5
#6  0x00007ffff281a0f0 in QApplication::notify(QObject*, QEvent*) ()
    at /usr/lib64/libQt5Widgets.so.5
#7  0x00007fffef2da245 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /usr/lib64/libQt5Core.so.5
#8  0x00007fffef2dc2a3 in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) () at /usr/lib64/libQt5Core.so.5
#9  0x00007fffef329043 in  () at /usr/lib64/libQt5Core.so.5
#10 0x00007fffe9494134 in g_main_context_dispatch ()
    at /usr/lib64/libglib-2.0.so.0
#11 0x00007fffe9494388 in  () at /usr/lib64/libglib-2.0.so.0
#12 0x00007fffe949442c in g_main_context_iteration ()
    at /usr/lib64/libglib-2.0.so.0
#13 0x00007fffef32888c in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () at /usr/lib64/libQt5Core.so.5
#14 0x00007fffef2d86ab in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () at /usr/lib64/libQt5Core.so.5
#15 0x00007ffff29eb68f in QDialog::exec() () at /usr/lib64/libQt5Widgets.so.5
#16 0x00000000005eb0d0 in ImportExport::ImportDialog::exec(ImportExport::KimFileReader*, QUrl const&) ()
#17 0x00000000005d3dbf in ImportExport::Import::exec(QString const&) ()
#18 0x00000000005d40f9 in ImportExport::Import::imageImport(QUrl const&) ()
#19 0x00000000005d4335 in ImportExport::Import::imageImport() ()

For that matter, a lot of times when I'm doing an import I'm really
not interested in selecting which files I want to import; just grab
the whole thing and go.
-- 
Robert Krawitz                                     <rlk at alum.mit.edu>

***  MIT Engineers   A Proud Tradition   http://mitathletics.com  ***
Member of the League for Programming Freedom  --  http://ProgFree.org
Project lead for Gutenprint   --    http://gimp-print.sourceforge.net

"Linux doesn't dictate how I work, I dictate how Linux works."
--Eric Crampton

More information about the Kphotoalbum mailing list