#1649 Cache autocomplete window for GTK+ platform

Bug
closed-fixed
5
2014-09-30
2014-08-29
No

Scintilla creating so many new windows on X11 causes problems. There are bugs like this one that cause XServer collisions that have been observed with Geany/Scintilla and with GTK3 sometimes it causes windows to be lost.

Depending on the computer/system sometimes this can cause severe performance impacts also. For example, on my computer with Geany built for GTK3, after a lot of use (with lots of "tags" that cause many autocompletion windows), it can cause window creation such as opening dialogs to take many seconds. I believe it's a bug/problem in XServer or GDK that is only exposed more regularly because Scintilla is creating so many Windows.

Colomban Wendling has made changes to Scintilla platform to cache the autocomplete window for GTK+ platform which I have been using for a couple of weeks and it seems to solve the XID collision problems (not experienced by me) and the massive slowdowns on new window creation that I was experiencing. I will contact Colomban to see about posting the patch for review and/or adding additional information.

Disclaimer: I'm making the assumption that too many new X windows is causing the problem, but it's completely unproven, all I can be sure is that with Colomban's cached window patch, my performance issues go away. I'm fairly certain it's a GTK/GDK/X11 bug that can be worked around fairly simple.

Discussion

  • Colomban Wendling

    Attached is the patch in question.

    As @codebrainz mentioned, since Geany started using GTK3 we started to sometimes (not often, but like maybe once every 2 weeks) get "dead" completion windows, together with an XID collision warning.

    Investigation of the issue didn't suggest any different code paths under GTK 2 or 3 in either Geany or Scintilla that could have anything to do with it, so our best guess is that unrelated GTK3 changes (like the fact creating new windows seems 5 times slower with gtk3 than 2) lead to an unlikely race condition to actually happen. The linked reports (and https://bugzilla.gnome.org/show_bug.cgi?id=581526) IIUC suggest that the issue is that an XID could be reused for a new window before the destroy notification for the previous one reaches the app.

    The attached patch always re-uses the same window, which will theoretically lower the odds for that issue to happen so drastically it's unrealistic to think it could actually ever happen. Moreover, a stress test for Geany seems to suggest the bug doesn't happen anymore with the patch.

     
    • Neil Hodgson

      Neil Hodgson - 2014-08-31

      The Platform interface is supposed to remain stable and should not break even binary compatibility unless really needed. Switching a method to virtual adds it to the vtable, potentially moving other entries and breaking implementations on other platforms. Instead use a private protocol (using something like g_object_set_data) between PlatGTK's Window and ListBoxX classes so that Window::Destroy can recognize a ListBoxX.

       
      • Colomban Wendling

        OK the code is less pretty then but should work just as fine.

        Here are two patches implementing it: the first (3) uses dynamic_cast to recognize ListBox instances, but as I don't know if you accept use of it, there's also the second one (4) that uses g_object_set_data() to tag Window::wid.

        I also have another patch that instead of moving all the logic inside Window::Destroy() "simply" calls a new ListBoxX::Hide(), but as this requires moving the whole ListBoxX class declaration above the implementation for Window::Destroy() I don't include it here. If you prefer it and don't mind the patch moving quite a bunch of code a few lines above, I can post it (and modify it to use your preferred implementation as asked above).

         
  • Neil Hodgson

    Neil Hodgson - 2014-09-02
    • labels: gtk gtk3 autocomplete xid collision trouble ahead --> gtk gtk3 autocomplete xid collision trouble ahead, scintilla, gtk
    • status: open --> open-fixed
    • assigned_to: Neil Hodgson
     
  • Neil Hodgson

    Neil Hodgson - 2014-09-02

    Committed second dynamic_cast patch as [67c4bd].

    Compilation will break if run-time type information is turned off (-fno-rtti added to the build flags) but that isn't in the makefile for GTK+ and its unlikely anyone will choose that now.

     

    Related

    Commit: [67c4bd]

  • Neil Hodgson

    Neil Hodgson - 2014-09-30
    • status: open-fixed --> closed-fixed
     

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks