GNOME Bugzilla – Bug 311740
GimpDataFactory should reload only modified files
Last modified: 2005-10-31 11:29:37 UTC
Per suggestion of Sven Neumann on the IRC, GimpDataFactory should reload only the files that were modified. Namely, a GimpData sould keep track of the mtime, keep the files in a cache upon refreshing, and load only the files whose mtime is newer. A Patch would be attached to this bug to implement this suggestion.
Created attachment 49846 [details] [review] A Patch to implement the suggestion. This is a patch to implement the suggestion. It still duplicates a lot of code and so should not be applied yet.
Created attachment 50863 [details] [review] A revised patch to implement the suggestion This is a revised patch that implements the suggestion. This time, the code of the original patch was refactored so it will be more modular. It still has a few rough spots like inconsistent function and variable naming (plus possibly some style problems). However, as a general rule it is ready for prime time.
Created attachment 53112 [details] [review] cleaned up version of the above patch Here's a revised version of that patch. It fixes a couple of coding style issues. Seems to work just fine but I haven't yet understood why the cache consists of a hash table of lists of data items. Shouldn't a hash table of data items be sufficient? But perhaps I just missed something here...
Sven: well, from what I understood there can be items with the same filenames, each in different directories in the data search path (say global data and user data). That's why each filename points to a list of such items.
Since the filenames are absolute filenames they are unique. The code would become quite a bit simpler if it would make use of this fact.
Hmm, there are indeed data objects sharing a same filename. There are a few formats that provide multiple data objects from a single file. SVG gradients are an example.
Created attachment 53464 [details] [review] shorter version of the patch This version of the patch removes code duplication that the previous versions had introduced. The function to actually load the data is now shared between the load and the reload implementations. IMO this can go into CVS now, but I haven't got around to give it some thorough testing.
I can crash this code by modifying a data file on disk and reloading the data. Before the crash the following critical assertion is triggered: g_object_unref: assertion `G_IS_OBJECT (object)' failed
+ Trace 63592
Shlomi, can we expect you to have a look at this problem?
Sven: hi! Sorry for the late response. I'm quite busy right now in many other endeavours (including a day job), and so cannot guarantee I'll investigate this problem soon. You can try tackling it on your own, or otherwise wait for me to have some spare cycles to look at it. Thanks for all your work on the patch so far.
I think I fixed the problem by simplifying the code even further. The cache is now kept around unmodified until the refresh operation is finished. Only at that point it is cleared. Closing as FIXED. 2005-10-31 Sven Neumann <sven@gimp.org> * app/core/gimpdata.[ch] * app/core/gimpdatafactory.c: applied a heavily modified version of the patch provided by Shlomi Fish in bug #311740. Introduces a cache to speed up reloading of data files. * app/actions/data-commands.c: set gimp busy while refreshing data factories.