GNOME Bugzilla – Bug 786504
GtkSourceMap causes performance issues for gedit
Last modified: 2017-08-21 18:15:26 UTC
Created attachment 357964 [details] [review] possible patch for lazy initializing the GtkSourceMap There is this Gedit bug: https://bugzilla.gnome.org/show_bug.cgi?id=172099 To make a very short summary, if you try to load a big text file in Gedit, it takes very long. I investigated the issue and one of the problems is that the gedit-view-frame contains a GtkSourceMap. This map is connected to the main view even if it's not visible (https://github.com/GNOME/gedit/blob/master/gedit/resources/ui/gedit-view-frame.ui#L34 -> which leads to the execution of the following line https://github.com/GNOME/gtksourceview/blob/master/gtksourceview/gtksourcemap.c#L1232). So if you load a file it is loaded also in the GTKSourceMap, even if the GTKSourceMap is never shown. This doubles the loading time. A possible solution would be to lazy initialize the GtkSourceMap (connect the view to the map only when the map is actually shown). This could probably be done in Gedit, but I was wondering if it wouldn't be better to do it in GtkSourceMap. I posted a possible patch for that. P.S. : If you apply just this patch, the gedit load time won't drop. A small gedit change is also needed. I can provide it if necessary. Thanks
Can you provide in the bug for gedit the patch for gedit? At first sight I don't see the point for this patch, but it would be much easier to test and understand it with the complete picture (taht is also the patch for gedit) Codewise, IMO your patch is un-necessarilly long. Instead of changing view to connected_view, it would be easier to mantain the variable and use another name for the "unconnected_view". With this, your patch would be much shorter, and readibility would not hurt. But, do not take this as a review as I fail to see yet whether this is the right approach (and I am not the maintainer of GtkSourceView)
Created attachment 357995 [details] [review] possible patch for lazy initializing the GtkSourceMap
Created attachment 357996 [details] [review] Gedit patch for using the GTKSourceMap lazy init
I added the gedit patch to this bug as well, just to have everything in one place. It's not meant for a review, just for providing a way to test the changes to the GTKSourceMap. Also I updated the GTKSourceMap patch since the old one had a problem.
The widget was designed to be thrown away when not used (we remove them when unused in Builder). That said, I don't actually see what about this patch should improve performance. The part that should be slowing things down is assigning the buffer to the view (the GtkSourceMap is a GtkSourceView after all). As it's going to be listening to the insert-text internally to update the view. It will actually be worse than 2x for larger buffers, because the visible area is larger than the normal view (and therefore has to pixel cache about 2x the height of the visible area). That means increased line validation, font sizing, layout, etc. If you want to fix it in GtkSourceMap (instead of just removing the map when unused) in gedit, you probably want to delay the assigning of the buffer to the GtkSourceMap until after the buffer has loaded. Unfortunately, GtkSourceBuffer doesn't have a loaded signal, so we don't exactly know when that has happened in a generic fashion. (Although, maybe we can add some hooks if GtkSourceFileLoader is being used).
Oh, I should also add, you can speed up the font metrics a slight amount by using a simpler font to size. We have one called BuilderBlocks in Builder which is a simple glyph that is easy to render/size/etc. https://git.gnome.org//browse/gnome-builder/tree/data/fonts/ https://git.gnome.org/browse/gnome-builder/tree/libide/editor/ide-editor-view.c#n58 That annoying/difficult thing here is we have to install the font to disk so we can use a custom FcFontMap to map all fonts to the BuilderBlocks font.
OK I think it's better to fix the problem in gedit then.
Thank you for the sugestions !
Surely GtkSourceView caches font metrics?
(In reply to George White from comment #9) > Surely GtkSourceView caches font metrics? Even if it cached the PangoFontMetrics, you'd still have to size the text to determine things like line-wrapping and the natural width of the view. When you are in the layout cycle, you are forced to do this manually for the clip window and spin a bit in a high-priority idle. Some of this could be optimized by adding a character index to the GtkTextBtree, but given the variability in font on a per-character basis in a textview, that's an awful lot of corner cases to handle. I hold the current viewpoint that we probably want to make a new code widget for Gtk 4 that does not inherit from GtkTextView, thus allowing significant optimizations not allowed by the single line-based BTree we have today.