[KPhotoAlbum] Import crash

[Robert Krawitz]

On Mon, 15 May 2017 22:46:55 +0200, Johannes Zarl-Zierl wrote:
> Hello Robert,
>
> Disclaimer: I don't have much time on my hand currently, so I can
> only review/ approve/merge patches. Sorry for that...

Very well, I guess I'l have to look at the code...

> On Montag, 15. Mai 2017 09:31:41 CEST Robert Krawitz wrote:
>> On Mon, 15 May 2017 09:06:04 -0400 (EDT), Robert Krawitz wrote:
>> > On Mon, 15 May 2017 09:02:05 -0400 (EDT), Robert Krawitz wrote:
>> >> I'm trying to import a huge number of photos (as a way of syncing up
>> >> two image stores), and kpa crashed.  "Huge number" in this case means
>> >> 90,000; I tried a similar exercise with a much smaller number with no
>> >> trouble.
>
> That is quite an impressive number, if I may say so.

Yeah, I've never synced the two copies of my image repository.

>> it happens after clicking Finish.
>> However, I was able to find the recursion.  It appears to actually be
>> recursion between copyNextFromExternal() and aCopyJobCompleted(); the
>> compiler notes the possibility for tail recursion from aCopyJobCompleted().
>
> 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...

Yes, they are.  They've been imported long since, but I haven't synced
the kpa metadata.

>> 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 ;-)

If I can reproduce it.

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

That would be a lot cleaner.

>> 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...
>
>
> Could you please open bug reports for these issues? Or better yet, provide patches ;-)

If I have time, I'll take a look at them.
-- 
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