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 169792 - let GScanner mmap() files
let GScanner mmap() files
Status: RESOLVED WONTFIX
Product: glib
Classification: Platform
Component: general
unspecified
Other All
: Normal enhancement
: ---
Assigned To: gtkdev
GIMP Bugs
Depends on:
Blocks:
 
 
Reported: 2005-03-10 01:24 UTC by Sven Neumann
Modified: 2005-03-15 16:28 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
let GimpScanner mmap() files if possible (4.44 KB, patch)
2005-03-10 01:26 UTC, Sven Neumann
none Details | Review
patch that makes GScanner mmap the input file if possible (3.95 KB, patch)
2005-03-10 17:48 UTC, Sven Neumann
rejected Details | Review

Description Sven Neumann 2005-03-10 01:24:58 UTC
Since using mmap seems to be en vogue at the moment and I wanted to see if it
makes a difference, I changed libgimpconfig/gimpscanner.c so it transparently
tries to use mmap() to read the file. Seems to work fine but it also doesn't
seem to make any measureable (or let alone noticeable) difference.

I had a look at the libgsf implementation of this and my implementation should
be as portable, even includes the code for Win32. However since it deosn't seem
to make any difference, I wonder if we should add this to CVS at all. It only
makes the code more difficult to maintain.
Comment 1 Sven Neumann 2005-03-10 01:26:09 UTC
Created attachment 38484 [details] [review]
let GimpScanner mmap() files if possible
Comment 2 Sven Neumann 2005-03-10 01:27:09 UTC
If anyone wants to give this a try, perhaps on Win32, uncomment
GIMP_SCANNER_PROFILE to get some basic profiling data.
Comment 3 Manish Singh 2005-03-10 01:36:14 UTC
Our config files are so tiny, this shouldn't make any difference. I'd vote for
code simplicity.
Comment 4 Sven Neumann 2005-03-10 10:54:47 UTC
pluginrc easily becomes 400kB; not exactly what I'd call tiny. But then we
aren't copying this into memory. The only thing that this patch changes is that
GScanner doesn't need to allocate a read buffer of 4kB and can save a few calls
to read().
I think it would make sense to use mmap() here if GLib provided an abstraction
layer for it so that we wouldn't have to deal with the portability issues.
Comment 5 Sven Neumann 2005-03-10 17:46:46 UTC
It appears that this request doesn't make much sense in GIMP. If at all then it
is GScanner that should be using mmap(). I'll reassign this report to GLib and
attach a patch for GScanner.
Comment 6 Sven Neumann 2005-03-10 17:48:16 UTC
Created attachment 38519 [details] [review]
patch that makes GScanner mmap the input file if possible
Comment 7 Sven Neumann 2005-03-10 17:51:09 UTC
The patch is somewhat hackish (it uses a magic value for the file descriptor and
abuses scanner->buffer to store the mmap'ed address). It might even break apps
that access the "private" input_fd field in the GScanner struct.
Comment 8 Matthias Clasen 2005-03-10 18:05:58 UTC
May affect bug 165693
Comment 9 Ben Maurer 2005-03-13 21:21:21 UTC
Is gscanner doing a one time read of the file (ie, reading it from start to
finish). If so, it is probably inefficient to use mmap here. mmap only makes
sense for long term usages of a file.
Comment 10 Sven Neumann 2005-03-13 21:52:03 UTC
Usually, a user of GScanner will read the file from start to end. The only
advantage of using mmap here is to avoid allocating 4kB for the scanner's buffer
(and copying the data through this buffer).
Comment 11 Matthias Clasen 2005-03-14 03:20:13 UTC
The thing to be optimized here is probably the generation of a 400K rc file in
the first place...
Comment 12 Sven Neumann 2005-03-15 00:30:37 UTC
I don't think the GIMP pluginrc is a considerably weak spot. Caching this data
in an rc file tremendously cuts down startup time. But this is somewhat
unrelated here. As reported on the gtk mailing-list, people are using GScanner
to implement XML parsers and read files that are a lot larger than this one.

But feel free to close this bug as WONTFIX. It was just an experiment I did and
I wanted to stick the patch somewhere. Since it doesn't seem to make much of a
difference anyway, I fully understand if you don't consider it useful.
Comment 13 Ben Maurer 2005-03-15 03:29:26 UTC
imho, this is a wontfix. aobut the 4 kb buffer: just alloca it.
Comment 14 Ben Maurer 2005-03-15 03:31:31 UTC
imho, this is a wontfix. aobut the 4 kb buffer: just alloca it.
Comment 15 Ben Maurer 2005-03-15 03:50:39 UTC
Am playing around ignore this
Comment 16 Ben Maurer 2005-03-15 03:50:53 UTC
x