Discussion:
[KPhotoAlbum] *startup-performance* branch
Robert Krawitz
2018-08-18 16:01:21 UTC
Permalink
Oops, I really meant the startup performance branch; the
load-performance branch has already merged.
--
Robert Krawitz <***@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
Johannes Zarl-Zierl
2018-08-18 19:19:15 UTC
Permalink
Hi Robert,

This week I had a first look at it. I'm not on my PC right now and therefore can't look at the patches for writing this response, but my initial thoughts were:

I'd like a better name for the doDirty() method (but I couldn't come up with one right away).

The trick of shortcutting the MemberMap.contains() lookup seems OK, but I wanted to do a second reading to make sure I understood it correctly.

On that note: since the lookup is only updated during m_loading time: can it get incorrect results e.g. by removing (sub)categories and then re-adding some of them?

Cheers,
Johannes
Post by Robert Krawitz
Oops, I really meant the startup performance branch; the
load-performance branch has already merged.
Robert Krawitz
2018-08-20 00:21:26 UTC
Permalink
Post by Johannes Zarl-Zierl
This week I had a first look at it. I'm not on my PC right now and
therefore can't look at the patches for writing this response, but
I'd like a better name for the doDirty() method (but I couldn't come
up with one right away).
How about markDirty()?
Post by Johannes Zarl-Zierl
The trick of shortcutting the MemberMap.contains() lookup seems OK,
but I wanted to do a second reading to make sure I understood it
correctly.
This is only done when loading; otherwise, the logic is the same as it
was before.
Post by Johannes Zarl-Zierl
On that note: since the lookup is only updated during m_loading
time: can it get incorrect results e.g. by removing (sub)categories
and then re-adding some of them?
I've fixed the remove code to always rebuild the flat list to avoid
issues in that regard.

I am assuming that nothing ever gets deleted while loading; perhaps I
should add an assertion to that effect?
--
Robert Krawitz <***@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
Johannes Zarl-Zierl
2018-09-13 21:36:41 UTC
Permalink
Hi Robert,

Thanks for the patch (and your patience!)...
Post by Robert Krawitz
Post by Johannes Zarl-Zierl
I'd like a better name for the doDirty() method (but I couldn't come
up with one right away).
How about markDirty()?
Fine by me...
Post by Robert Krawitz
I am assuming that nothing ever gets deleted while loading; perhaps I
should add an assertion to that effect?
Adding an assertion wouldn't hurt - although I can't fathom how we ever could
end up breaking that assertion...

I've merged the patchset in its current form.

Cheers,
Johannes

Loading...