After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 330099 - Startup: Data should be loaded and unloaded in a lazy manner
Startup: Data should be loaded and unloaded in a lazy manner
Status: RESOLVED OBSOLETE
Product: GIMP
Classification: Other
Component: Data
2.8.6
Other All
: High major
: Future
Assigned To: GIMP Bugs
GIMP Bugs
: 526122 543940 557864 KainPlan 608313 682348 704135 792313 (view as bug list)
Depends on:
Blocks: 141797
 
 
Reported: 2006-02-06 11:10 UTC by Shlomi Fish
Modified: 2018-05-24 11:43 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
The relevant patch - revision 9. (79.02 KB, patch)
2006-02-06 11:12 UTC, Shlomi Fish
none Details | Review
The relevant patch - revision 12 (78.48 KB, patch)
2006-02-06 19:42 UTC, Shlomi Fish
none Details | Review
Patch revision 13 (79.39 KB, patch)
2006-02-07 11:06 UTC, Shlomi Fish
none Details | Review
Patch revision 14 (78.92 KB, patch)
2006-02-07 14:56 UTC, Shlomi Fish
needs-work Details | Review

Description Shlomi Fish 2006-02-06 11:10:16 UTC
Attached to this bug is the recent version of my patch to make sure that data in the GIMP is loaded and unloaded in a lazy manner. This patch adds two function to GIMP_DATA - gimp_data_load() and gimp_data_unload() for signifying when the data of the object should be loaded to memory. This patch makes sure that only the essential information for the data is loaded, and the rest is loaded from the disk when it is requested.
Comment 1 Shlomi Fish 2006-02-06 11:12:23 UTC
Created attachment 58796 [details] [review]
The relevant patch - revision 9.

This is revision 9 of the patch. It is possible that some loading functions for sub-types of data were not converted yet.
Comment 2 Shlomi Fish 2006-02-06 19:42:06 UTC
Created attachment 58822 [details] [review]
The relevant patch - revision 12

This is a newer version of the patch. It has a problem that when GIMP exits it generates a lot of warnings. I'll try to investigate further.
Comment 3 Shlomi Fish 2006-02-07 11:06:25 UTC
Created attachment 58850 [details] [review]
Patch revision 13

This patch eliminates the warnings that appeared in revision 12, and also fixes the bugs that caused them.
Comment 4 Shlomi Fish 2006-02-07 14:56:55 UTC
Created attachment 58865 [details] [review]
Patch revision 14

This new revision of the patch fixes many GError-related problems. Thanks to Sven for pointing me one of them.
Comment 5 Sven Neumann 2006-02-07 15:02:36 UTC
fprintf (stderr, foo) must not be used. Printing error messages to stderr is fine for debugging and perhaps for the impossible error, but it should not be used in general because the casual user will never see any error message printed there.

If you want to print error messages to stderr, please use g_printerr() instead. g_printerr() takes care of doing the charset conversion to the user's locale in case it isn't an UTF-8 locale.
Comment 6 Michael Natterer 2006-02-13 13:19:26 UTC
Finally, some comments :-)

- GimpBrush::load/unload() should probably be called data_load()
  and data_unload() because that's how the functions are called
  and it makes the purpose clearer.
- gimp_data_load()/unload() should be called use() and unuse() because
  that's what they do, changing the use count and not loading/unloading.
- the GimpBrushType enum should be typedefed as all out enums, live
  in app/core-enums.h and used as such in the GimpBrush struct
  (instead of gint)
- instead of g_error_free(error), it's safer to use g_clear_error(&error)
  because it sets the error variable to NULL so it's ready for being
  reused.
- gimp_data_unload() needs g_return_if_fail(load_count > 0)
- in order to jump in one's eyes, the TODOs in the code should be:
  #ifdef __GNUC__
  #warning FIXME foobar
  #endif
- there are still some incomplete prototypes
- some function headers are indented like prototypes
- please let's get rid of these perl utility functions in the pdb files
  and put the duplicated code back. pdbgen files are hard enough to
  read already.
  (e.g. _get_gradient_code() and friends)
Comment 7 Sven Neumann 2006-02-13 15:04:49 UTC
The patch introduces file offsets and I don't really understand how this is supposed to work. There seems to be an assumption made here that the files on disk would not change. This is a dangerous assumption to make as of course the data files may be erased or changed while GIMP is running. The data files would thus have to be reloaded completely. Of course this might fail. So the approach we are currently taking introduces a lot of error handling. Currently we can assume the integrity of the data objects in all places. The data can only change or vanish if the user hits the Reload button. Perhaps it isn't such a good idea to change this.

So I would like to propose another approach which might turn out to be easier to get right. The idea is to use something very similar to the GTK+ icon cache. For each of the data directories in a data types search path, there would be a data cache on disk. This cache file would contain the data of all data files in that particular directory. It would contain this data in a form that can directly be mmap'ed (using GMappedFile).  On startup and whenever the user hits Reload, GIMP would check the integrity of the cache file by comparing the stored file sizes and modification times to the files on disk. If a file on disk has been changed, deleted or added, the cache is regenerated. For the code in gimp this means less changes because all users of GimpData could continue to assume that the data is there and loaded. We would still reduce our memory footprint and startup time because all the data files would just be mmap'ed. In order to reduce the time for the very first start, the global data directories could already contain data cache files that are generated at installation time.
Comment 8 Manish Singh 2006-02-13 18:38:29 UTC
Another point of reference would be to look at fontconfig HEAD. They've implemented cache files via mmap.
Comment 9 weskaggs 2006-05-21 19:35:17 UTC
It seems that there are too many issues remaining for this to be implemented in 2.4, which is going to be released very shortly, so I am bumping the target to Future.  Hopefully the not-too-distant-Future.
Comment 10 Sven Neumann 2008-04-04 10:47:56 UTC
*** Bug 526122 has been marked as a duplicate of this bug. ***
Comment 11 Michael Schumacher 2009-02-10 21:37:23 UTC
*** Bug 543940 has been marked as a duplicate of this bug. ***
Comment 12 Michael Schumacher 2009-02-11 21:57:46 UTC
*** Bug 557864 has been marked as a duplicate of this bug. ***
Comment 13 Michael Schumacher 2009-09-07 08:17:12 UTC
*** Bug 594349 has been marked as a duplicate of this bug. ***
Comment 14 Michael Schumacher 2010-01-28 10:48:30 UTC
*** Bug 608313 has been marked as a duplicate of this bug. ***
Comment 15 Michael Schumacher 2012-08-21 13:06:38 UTC
*** Bug 682348 has been marked as a duplicate of this bug. ***
Comment 16 Qua 2012-08-21 18:08:21 UTC
So, does this patch work with 2.8? It looks like it's old. Or is this just an issue you are currently trying to just implement in a future version?
Comment 17 Shlomi Fish 2012-08-22 07:35:45 UTC
(In reply to comment #16)
> So, does this patch work with 2.8? It looks like it's old. Or is this just an
> issue you are currently trying to just implement in a future version?

I have no idea if this patch works with 2.8. Maybe it does not. In any case, I have written it for a much older version, and it was deemed unsuitable due to the fact that I didn't use mmap() and I didn't write a new version based on mmap().
Comment 18 Max Mustermann 2013-07-13 07:13:23 UTC
*** Bug 704135 has been marked as a duplicate of this bug. ***
Comment 19 Max Mustermann 2013-07-13 07:21:07 UTC
We have many bug reports on this. Often Windows users report that GIMP takes so long to startup that Windows assumes GIMP is dead and reports a crash 
As this is not a pure nice-to-have-enhancement, but an old, known problem and there has been existing a patch for more than seven years, we should really make the patch matching to the latest API and apply it. Thus increasing the severity and setting the milestone to 2.8.
Comment 20 Thomas Hotz 2013-07-13 08:42:55 UTC
May I ask you a question? Sven, there was a call that GIMP is searching Windows developers to improve the experience of the Windows version. Have there been any afforts?
Comment 21 Max Mustermann 2013-07-13 09:04:21 UTC
Of course you may ;-)
I heard that call, too, and decided to support the GIMP help team here. In the meantime a lot of tasks has come up and fills me, like maintenance of the continuous build server (Jenkins), wiki work, testing, bug reporting and triaging and mailing list discussions. So, I'm personally quite busy and the work I originally wanted to do is having a slow progress. (Yes, I'm a bit sad about it)
There were some other short timed attempts to contribute Windows-related code to GIMP which seem to have already ended. In the meantime some other hopeful contributors appeared, for instance Téo and Jehan, who are working on GIMP in general (at least their efforts also benefit the Windows users).
To summarize it: yes, we have done something to improve the situation for Windows, but still any help is welcome.
Comment 22 Jehan 2013-10-03 14:56:08 UTC
Hello,

2 questions:

1/ so is the proposed right implementation supposed to be done with mmap then?

2/ I see you are also writing about slowness on Windows. But isn't mmap a UNIX/POSIX API? If so, the lazy-loading implementation would not pertain to the Windows version anyway. Or I misunderstood something?

Thanks.
Comment 23 Michael Natterer 2016-04-20 11:24:36 UTC
This is clearly not making it into 2.8, and not into 2.10 either unless
some miracle happens.
Comment 24 Michael Natterer 2018-01-07 23:35:30 UTC
*** Bug 792313 has been marked as a duplicate of this bug. ***
Comment 25 GNOME Infrastructure Team 2018-05-24 11:43:56 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gimp/issues/178.