[KPhotoAlbum] Bug in image cache size computation

[Robert Krawitz]

The number of items in the image cache is computed incorrectly if the
image cache size is 2048 MB or greater; it's effectively zero due to
signedness problems. Performance is much better with this fix.

Also, why the hard upper limit of 4096 MB? I have 16 GB of RAM in my
laptop, and might as well take advantage of it.

diff --git a/Viewer/ImageDisplay.cpp b/Viewer/ImageDisplay.cpp
index 1775efb..77be9a9 100644
--- a/Viewer/ImageDisplay.cpp
+++ b/Viewer/ImageDisplay.cpp
@@ -567,7 +567,7 @@ void Viewer::ImageDisplay::setImageList( const DB::FileNameList& list )

void Viewer::ImageDisplay::updatePreload()
{
- const int cacheSize = ( Settings::SettingsData::instance()->viewerCacheSize() * 1024 * 1024 ) / (width()*height()*4);
+ const int cacheSize = (int) ((long long) ( Settings::SettingsData::instance()->viewerCacheSize() * 1024ll * 1024ll ) / (width()*height()*4));
bool cacheFull = (m_cache.count() > cacheSize);

int incr = ( m_forward ? 1 : -1 );

[Henner Zeller]

Post by Robert Krawitz
The number of items in the image cache is computed incorrectly if the
image cache size is 2048 MB or greater; it's effectively zero due to
signedness problems. Performance is much better with this fix.
Also, why the hard upper limit of 4096 MB? I have 16 GB of RAM in my
laptop, and might as well take advantage of it.
diff --git a/Viewer/ImageDisplay.cpp b/Viewer/ImageDisplay.cpp
index 1775efb..77be9a9 100644
--- a/Viewer/ImageDisplay.cpp
+++ b/Viewer/ImageDisplay.cpp
@@ -567,7 +567,7 @@ void Viewer::ImageDisplay::setImageList( const DB::FileNameList& list )
void Viewer::ImageDisplay::updatePreload()
{
- const int cacheSize = ( Settings::SettingsData::instance()->viewerCacheSize() * 1024 * 1024 ) / (width()*height()*4);
+ const int cacheSize = (int) ((long long) ( Settings::SettingsData::instance()->viewerCacheSize() * 1024ll * 1024ll ) / (width()*height()*4));

fyi, it is a good idea to have long constants with uppercase L,
because the lowercase l is easily confused with a 1 (one) (Don't ask
me how I know :) ).

So I suggest 1024ll -> 1024LL

-h

Post by Robert Krawitz
bool cacheFull = (m_cache.count() > cacheSize);
int incr = ( m_forward ? 1 : -1 );
--
*** 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
_______________________________________________
KPhotoAlbum mailing list
https://mail.kdab.com/mailman/listinfo/kphotoalbum

[Miika Turkia]

Post by Robert Krawitz
The number of items in the image cache is computed incorrectly if the
image cache size is 2048 MB or greater; it's effectively zero due to
signedness problems. Performance is much better with this fix.
Also, why the hard upper limit of 4096 MB? I have 16 GB of RAM in my
laptop, and might as well take advantage of it.
diff --git a/Viewer/ImageDisplay.cpp b/Viewer/ImageDisplay.cpp
index 1775efb..77be9a9 100644
--- a/Viewer/ImageDisplay.cpp
+++ b/Viewer/ImageDisplay.cpp
@@ -567,7 +567,7 @@ void Viewer::ImageDisplay::setImageList( const DB::FileNameList& list )
void Viewer::ImageDisplay::updatePreload()
{
- const int cacheSize = ( Settings::SettingsData::instance()->viewerCacheSize() * 1024 * 1024 ) / (width()*height()*4);
+ const int cacheSize = (int) ((long long) ( Settings::SettingsData::instance()->viewerCacheSize() * 1024ll * 1024ll ) / (width()*height()*4));
bool cacheFull = (m_cache.count() > cacheSize);

You really had me fooled with this one. Not easy at all to decipher,
what is going on before getting the morning fix of caffeine. Anyway,
you could as well change to unsigned cacheSize while you are at it.
But that would probably take a bit more effort to change on the
required other places.

miika

[Robert Krawitz]

Post by Miika Turkia

Post by Robert Krawitz
The number of items in the image cache is computed incorrectly if the
image cache size is 2048 MB or greater; it's effectively zero due to
signedness problems. Performance is much better with this fix.
Also, why the hard upper limit of 4096 MB? I have 16 GB of RAM in my
laptop, and might as well take advantage of it.
diff --git a/Viewer/ImageDisplay.cpp b/Viewer/ImageDisplay.cpp
index 1775efb..77be9a9 100644
--- a/Viewer/ImageDisplay.cpp
+++ b/Viewer/ImageDisplay.cpp
@@ -567,7 +567,7 @@ void Viewer::ImageDisplay::setImageList( const DB::FileNameList& list )
void Viewer::ImageDisplay::updatePreload()
{
- const int cacheSize = ( Settings::SettingsData::instance()->viewerCacheSize() * 1024 * 1024 ) / (width()*height()*4);
+ const int cacheSize = (int) ((long long) ( Settings::SettingsData::instance()->viewerCacheSize() * 1024ll * 1024ll ) / (width()*height()*4));
bool cacheFull = (m_cache.count() > cacheSize);

You really had me fooled with this one. Not easy at all to decipher,
what is going on before getting the morning fix of caffeine. Anyway,
you could as well change to unsigned cacheSize while you are at it.
But that would probably take a bit more effort to change on the
required other places.

I tried that, but that did have some upstream problems, at least one
related to Qt (QMap.count() returns an int, believe it or not) and I
didn't take it any further. Another, perhaps better, option would be
to make cacheSize a long long.