Project

General

Profile

Bug #9550

cache_memory is reset to zero (with bad effects) if set to values >=2048

Added by Igor Kuzmin almost 7 years ago. Updated over 6 years ago.

Status:
Fixed
Priority:
Medium
Assignee:
Category:
General
Target version:
-
Start date:
08/05/2013
Due date:
% Done:

100%

Estimated time:
Affected Version:
git development version
System:
all
bitness:
64-bit
hardware architecture:
amd64/x86

Description

cache_memory option in preferences dialog is now set in MB and unlimited. In theory this should allow setting it to values higher than 2GB. In practice only prefs code uses bigger integer type (int64) while code that actually checks this setting still has hardcoded max value of 2GB and uses 32bit integer type (see src/common/mipmap_cache.c:640). When I try setting cache_memory to something like 2048 or 4096 it gets reset to zero and in result after restart all thumbnails are constantly being regenerated. We should either limit maximum value of this setting in preferences dialog (and revert to int32 type), or better yet fix the mipmap cache code to use 64bit integers and not to clamp this setting.

History

#1 Updated by Pascal Obry over 6 years ago

Ok, can you tell me if you are on a 64 or 32 bit machine?

Also, the code you're pointing is:

uint32_t max_mem = CLAMPS(dt_conf_get_int("cache_memory"), 100u<<20, 2u<<30);

So, it is an unsigned integer and it should be supporting up to 4gb I think. Maybe an issue in another place.

#2 Updated by Pascal Obry over 6 years ago

Hum... I'm wondering if this is not only an issue with the preference in the GUI. Can you edit ~/.config/darktable/darktablerc and set cache_memory value to 2048000000, does that also reset the cache?

#3 Updated by Pascal Obry over 6 years ago

Actually there is a bug in the preference but also in the code. The later is fixed in a branch and the former will need some more time.

#4 Updated by Tobias Ellinghaus over 6 years ago

  • System changed from other GNU/Linux to all
  • Affected Version changed from git stable branch to git development version
  • % Done changed from 0 to 20
  • Status changed from New to Triaged

dt_conf_get_int() returns an int, so the unsigned doesn't help. We should probably use dt_conf_get_int64 as that is the type in the config, too.

#5 Updated by Johannes Hanika over 6 years ago

should we just change it to also accept sizes in MB instead of bytes? backward compatibility could probably assured by non-overlapping sane ranges (16000 MB is a lot, bit still not enough for a cache if it was in bytes).

#6 Updated by Tobias Ellinghaus over 6 years ago

Just FYI: the cache_memory setting already uses a mechanism to display megabytes in the gui while still having the old byte value in the config. For details see data/darktableconfig.xml.in and grep for cache_memory.

#7 Updated by Pascal Obry over 6 years ago

This fix is in the extend-cache-upper-limit branch on Git. If you are building from source you can try it and tell us if this is working on your side.

#8 Updated by Pascal Obry over 6 years ago

  • % Done changed from 20 to 100
  • Priority changed from Low to Medium
  • Assignee set to Pascal Obry
  • Status changed from Triaged to Fixed

Now fixed on master. Closing this ticket.

#9 Updated by Igor Kuzmin over 6 years ago

Indeed it seem to be fixed now. The only complaint I can add is that user still can set mipmap cache size to very high values and he will get no notice that it's actually limited to 8GB. I think it would be a good idea to set the same limits for input fields in preferences as they are in the code using that variable.

#10 Updated by Tobias Ellinghaus over 6 years ago

Makes sense, I will look into it.

Also available in: Atom PDF

Go to top