Tk Source Code

Running on Linux (8.6.9 and 8.6.10 at least), the various screens in my application take a very long time to build and draw.

Mac OS is much faster.

non-empirical guesstimates give about 7 seconds to display my options screen 
(very complex) on Linux, and 1 second on mac os, and perhaps 1+ seconds on 
windows.

This is with creating new widgets.  I do not destroy the widgets, so
displaying the screen a second time is not an issue.

Are there any known issues causing this slowdown on X11?

Using a stopwatch:
Linux: 11 seconds
Mac OS and Windows: < 1 second

@dkf,
Hi,
could you please help us with that bug?

thanks,
++
nicolas

Reported again, see [5d52ad92fa].

Attached patch for version 8.7

Thanks bb9.

François has resolved his thoughts about this patch.
He likes to have a second opinion / confirmation (it is rather 
difficult to write a test suite for this).  He's trying
to find someone to look at it.

Some slowdown will probably always be there (see the uim-xim timing below),
though it should not be too bad, and the Tk applications
do not keep slowing down as before with this patch.

I should add to my previous report
by "a simple (nearly empty) script"
I mean just "package require Tk" and exit
without explicitly creating any windows

so for some reason "package require Tk"
increased load time five fold with uim-xim

I know you asked about the callbacks when I wrote the first XIM patch.
But it must have been in an e-mail, it's not in the prior ticket.

The logic works like:

Register instantiation callback.
- X11 registers an IME, calls the instantiation code.
Call OpenIM
Register the destroy callback.
Un-register the instantiation callback.

- IME is closed. call the destroy callback.
Register the instantiation callback.

And it repeats ad infinitum as the IME goes away and gets restored.

OpenIM() only gets called when the instantiation callback is run,
and that's only run once when the IME is registered.   The instantiation
callback is unregistered at that time, as there can't be a second IME active,
and multiple calls to OpenIM() will not be made for an instance of an IME.

> XOpenIM() is called at exactly one place, in OpenIM(). Before the patch 
> OpenIM() was called from two places (in TkpOpenDisplay() and 
> InstantiateIMCallback(), and that indeed looks superfluous. It should be called 
> only once, in TkpOpenDisplay(). Brad, didn't you remove the wrong one? I mean, > it certainly works with the patch as it is, but instead of calling XOpenIM() 
> once per application it calls it for each window that needs to build an input 
> context, which still seems too much to me. 

XOpenIM must be called from InstantiateIMCallback.  If the input method
disappears (due to failure, restart, user kill, whatever (this is the 
segmentation fault I fixed before) ), the destroy callback is called, and
the instantiation callback is re-registered.

XOpenIM() is only called once for the application, and whenever the IM is 
restarted.

The code is correct as it is.

Informational:

Throughout this ticket, "window" refers to an X11 window.
A "package require Tk" creates 4 X11 windows.
The manywidgets.tcl script creates 1004 X11 windows.

The 'xrestop' program can be used to list X11 resources used by a process.

> Brad, didn't you remove the wrong one?

I will have to dig through everything again.  It's been quite a while
since I wrote this.

> The patch removes the TK_CHECKED_IC machinery.

I think I was just simplifying/removing unecessary stuff.

> The patch now calls CreateXIC() only for toplevel windows (that don't already have a winPtr->inputContext).

I don't even know that this is necessary.
I was worried about key bindings.
The XIC is probably not needed for top levels.

>> Why is the call to XOpenIM() commented out?
>
> As stated, there was a duplicate call to XOpenIM().

You must both mean OpenIM(), not directly XOpenIM().

XOpenIM() is called at exactly one place, in OpenIM(). Before the patch OpenIM() was called from two places (in TkpOpenDisplay() and InstantiateIMCallback(), and that indeed looks superfluous. It should be called only once, in TkpOpenDisplay(). Brad, didn't you remove the wrong one? I mean, it certainly works with the patch as it is, but instead of calling XOpenIM() once per application it calls it for each window that needs to build an input context, which still seems too much to me.

Apart from the above, what I gather from the patch is that:

• The patch now calls CreateXIC() only for toplevel windows (that don't already have a winPtr->inputContext). Previously it called it for every window (that don't already have a winPtr->inputContext). What makes us think this is enough? I suppose that some testing indicated that, say, an entry widget inside a toplevel has input methods still working with the patch, right? How can we be sure that doing this for toplevels is enough and that we didn't forget some kind of window? Is there any documentation detailing the fact that creating input contexts for toplevels is enough for letting input methods work in subwindows?

• However, the patch also now calls CreateXIC() on FocusIn event, which was not the case before. Is this change the answer to my above questions?

• The patch removes the TK_CHECKED_IC machinery. About this I'm wondering a bit. The patch seems to consider that checking winPtr->inputContext against NULL is the way to decide whether to create a new input context or not. I tend to agree with this but then why did someone previously introduce the TK_CHECKED_IC machinery in addition to that? Without the answer to that question I'm not sure we should remove it.

• Also, removal of the TK_CHECKED_IC flag happens in the public interface, in tk.h. Can we really do this in a patch release...? Perhaps after all. I fail to imagine a use case of this flag by an extension.

Attached an image of the patch working.  This is with ibus-daemon and mozc.

ll-g7:bll$ rlwrap ./linux/64/tcl/bin/tclsh
% package require Tk
8.6.10
% ttk::entry .e
.e
% pack .e
% 

I have had this patch in production for a long time.

> Why is the call to XOpenIM() commented out?

As stated, there was a duplicate call to XOpenIM().

> Note 1. macOS (when using Aqua) creates far fewer system-level windows, instead doing a lot of the work of clipping and so on itself. It's not a fair comparison at all. 

I never compared to MacOS.  The number of X11 windows active is 
important because the older code opened a XIC for *every* single X11 window, needed or not.  That's most of the time spent right there.

FWIW, I'd love to know what's going on during that 11 seconds; where is the time being spent? For something that gross, even the most basic of profiling should point to exactly the problem.

Reviewing the patch in context (bug-xim branch), I'm not convinced any more that this change will actually allow input methods to activate at all. According to what I've read, XOpenIM() should be called once (immediately after opening the display) and then when the set of actual input methods changes (the purpose of XRegisterIMInstantiateCallback()). Without that, XCreateIC() (the code to actually create an input method context, which I think is an on-focus thing AIUI) will get a NULL first argument, which might or might not work at all. (This is something about which the documentation is fundamentally unclear.)

It's not at all clear to me why things would be taking much longer on Linux unless we're calling XOpenIM or XCreateIC far more often than we ought to. Ideally that would be once per program (unless the user does something weird like changing the GUI locale dynamically) and once per non-crossing focus-in event, respectively.

Why is the call to XOpenIM() commented out? It's an explicit stated goal of Tk to support input methods, so in the absence of a very good justification, that alone would be a sufficient reason to reject the patch.

Note 1. macOS (when using Aqua) creates far fewer system-level windows, instead doing a lot of the work of clipping and so on itself. It's not a fair comparison at all.

Note 2. Input methods are the worst documented part of X11, at least in English.

This appears to be related to issue #1924761. I don't have a platform suitable for test this; XQuartz is its own special collection of weird.

To test with uim-xim:

sudo apt install uim-xim uim-anthy anthy
pkill ibus-daemon
uim-xim --engine=anthy-utf8

# in another terminal...
export XMODIFIERS=@im=uim
tclsh manywidgets.tcl

For the .XCompose issues, better to open a new ticket.
I don't know anything about that.

Supposedly, they will be releasing 8.6.11 in a couple of days, but
nobody except me and you care about this particular patch.

The bug-xim fix (there's a patch for 8.6.11 attached to this ticket) gives 
a 70% increase in speed using the attached manywidget test script when
using uim-xim.

( /usr/bin/tclsh is version 8.6.9 installed from the debian repositories. )

### speed differences between unpatched 8.6.9 and patched 8.6.10.

bll-g7:bll$ echo $XMODIFIERS
@im=uim
bll-g7:bll$ /usr/bin/tclsh mw.tcl
elapsed 1303
bll-g7:bll$ $HOME/local/bin/tclsh mw.tcl
elapsed 401
bll-g7:bll$ XMODIFIERS=@im=none
bll-g7:bll$ /usr/bin/tclsh mw.tcl
elapsed 60
bll-g7:bll$ $HOME/local/bin/tclsh mw.tcl
elapsed 24
bll-g7:bll$ XMODIFIERS=@im=uim
bll-g7:bll$ /usr/bin/tclsh mw.tcl
elapsed 1327
bll-g7:bll$ $HOME/local/bin/tclsh mw.tcl
elapsed 402
bll-g7:bll$ ../linux/64/tcl/bin/tclsh mw.tcl
elapsed 404

### note here how the unpatched tcl/tk gets slower and slower.
### whereas the patched version has a consistent speed.

bll-g7:bll$ /usr/bin/tclsh mw.tcl
elapsed 2399
bll-g7:bll$ /usr/bin/tclsh mw.tcl
elapsed 2419
bll-g7:bll$ ../linux/64/tcl/bin/tclsh mw.tcl
elapsed 402
bll-g7:bll$ ../linux/64/tcl/bin/tclsh mw.tcl
elapsed 402
bll-g7:bll$ ../linux/64/tcl/bin/tclsh mw.tcl
elapsed 404
bll-g7:bll$ /usr/bin/tclsh mw.tcl
elapsed 2494
bll-g7:bll$ /usr/bin/tclsh mw.tcl
elapsed 2568
bll-g7:bll$

amending my mistake
as GTK2 does work with the following

    GTK_IM_MODULE=xim XMODIFIERS="@im=none"

UIM input method also causes significant slow down (uim-xim)

a simple (nearly empty) script runs about five times slower
depending on the presence of the following command

    package require Tk

further, compose keys in ~/.XCompose are ignored with

    XMODIFIERS="@im=xim"

but not with (which unfortunately conflicts with GTK2)

    XMODIFIERS="@im=none"


tcl/tk 8.6.10
https://github.com/uim/uim/wiki/Uim-xim

Added patch for Tk 8.6.11

Check for TK_ALREADY_DEAD is put in place.

I have now installed ibus and I had a try with your test script.

With either core-8-6-branch or bug-xim, I do not reproduce the increase in the elapsed time that you are showing when invoking your test script repeatedly.

Moreover, with bug-xim I see a reported elapsed time about half of the one I get in core-8-6-branch, but this is very far from the performance increase that you are reporting with the bugfix branch.

Really, this problem and associated bugfix are not in my area of knowledge. I definitely think someone else should confirm this ticket and fix. The lines you're changing were committed by dkf, perhaps he could have a look.

Just a note based on reading your changes [4abb3952]: removal of the check on TK_ALREADY_DEAD in generic/tkEvent.c:1232 looks incorrect to me (just my 2 cents).

The bind manual page states "KeyPress and KeyRelease events are  sent  to
the window which currently has the keyboard focus.", so I think the way
the patch as implemented is fine.  A window must have the focus to process
a key event, and the XIC is created (if not already there) upon receiving
focus.

Test script attached.

I do not know why the response time slows down so dramatically.

This seems to be either less of a problem or not a problem with 
the bug fix in place.

bll-tecra:bll$ tclsh
% info patch
8.6.9
bll-tecra:bll$ tclsh manywidget.tcl
elapsed 5555
bll-tecra:bll$ tclsh manywidget.tcl
elapsed 5905
bll-tecra:bll$ tclsh manywidget.tcl
elapsed 8015
bll-tecra:bll$ tclsh manywidget.tcl
elapsed 10401
bll-tecra:bll$ tclsh manywidget.tcl
elapsed 12626

# restarted ibus-daemon
bll-tecra:bll$ tclsh manywidget.tcl
elapsed 3784

# turn IM off
bll-tecra:bll$ XMODIFIERS=@im=none
bll-tecra:bll$ tclsh manywidget.tcl
elapsed 117

# with the bug fix
bll-tecra:bll$ XMODIFIERS=@im=ibus
bll-tecra:bll$ ../linux/64/tcl/bin/tclsh manywidget.tcl
elapsed 65

I have just run the test suite on Debian 8 and indeed there are several test instabilities.

Besides, I'm not sure my setup has any input method enabled, I think it's needed to properly test your changes.

Oh well...

I will put a script together within the next few days.

If you can run the test suite and compare with-fix against the base source code,
that would be helpful.   I ran a couple of the test files. I often get
inconsistent results, but I think they are ok.

I just realized that the canvas bindings all need to be tested, text bindings,
anything with keyboard bindings.  And I have not tested the canvas or text widget.

I also have the thought in my mind that the bug fix could be changed to create
the XIC for any toplevel, "enter" event or "focus" event, and that would 
probably be more robust, and should still preserve the gain in responsiveness.

Well I could perhaps spend some time looking at this, despite this subject not being my cup of tea, but do you have a small script that I could time to show the time difference between before and after the fix?

@fvogel: For review.

Purpose of the bug-xim branch:
 
   Improve the responsiveness of Tk on X when creating many widgets.

Some notes:

echo $XMODIFIERS  # see what XMODIFIERS is set to
XMODIFIERS=@im=none    # turn off any input method
XMODIFIERS=@im=ibus    # set input method to use ibus-daemon

If the ibus-daemon does need to be restarted, use:
  ibus-daemon -d -r -x
There's also a -n argument to specify the window manager.  I don't know
if that's important.  My system uses: -n xfce

Timing:
  core-8-6-10 ;   XMODIFIERS=@im=ibus
     2027 milliseconds 
  core-8-6-10 ;   XMODIFIERS=@im=none
     316 milliseconds
  bug-xim ;   XMODIFIERS=@im=ibus
     285 milliseconds
  bug-xim ;   XMODIFIERS=@im=none
     281 milliseconds

xim-bug branch:
  a) There was an extra call being made to OpenIM(). 
     This was fixed.  This had no effect on the timing.

  b) Tk uses a *lot* of windows.  The timing test I have been running has
     545 windows total.  As comparison, xfwm4 has 122.

     I changed Tk_HandleEvent to only create and attach the XIC 
     to the window if (a) The window is a toplevel or (b) The window
     is receiving focus.  The XIC will remain intact thereafter.

1951 milliseconds to display, immediately after restarting the X server.
ibus daemon is running.

260 milliseconds to display, ibus daemon was killed (2nd run, no restart).

If I kill the ibus daemon, this problem goes away.

So this has to do with the XIM (X input method) stuff.

I will create a test script within the next couple of days.

This has more to do with some sort of X11 resource leak.  Rebooting the
machine helps a lot.