Re: [PATCH] system-type cygwin with window-system w32

[Lennart Borgman]

What happened to this? Is not this a very important problem?

On Mon, Jul 18, 2011 at 23:01, grischka <address@hidden> wrote:
> Daniel Colascione wrote:
>>>
>>> Actually the NT build works quite well with one single thread.
>>> Already tested here.
>>
>> Do you have a patch handy?
>
> Attached.  Patch was against state at commit:
>
>      Jan D. <address@hidden>  2011-02-01 09:53:03
>      Use add/delete_read_fd in xsmfns to simplify.  Also ...
>
> Hopefully still applies cleanly.
>
> It is minimal, and maybe not quite obvious.  Basically the two window
> message switches (w32fns.c and w32term.c) could be merged into one.
> The patch calls the one from the other.
>
> There is an elisp variable 'w32-single-thread' which should return 1
> if you want to make sure you're using the right build. :)
>
> Note that you can for example drag the "Open File" dialog across the
> main window and it still gets redrawn.
>
>>> * peek for messages in the QUIT macro (say via ELSE_PENDING_SIGNALS)
>>>  which is for C-g to interrupt lisp.
>>
>> I don't think this approach alone would be sufficient.  I may be wrong,
>> but I think the Windows window manager will consider a program to be
>> "unresponsive" if it stops actually pumping messages; I don't think
>> peeking is sufficient.  Also, [1] says that we shouldn't delay pumping
>> messages even if we're able to guarantee we'll get around to them later.
>> (Running lisp code is indistinguishable from sleep in this case.)
>
>> Using PeekMessage in lisp code instead of actually pumping messages
>> would be like telling your credit card company, "Yeah, I got the bill,
>> and I'll get around to responding sometime in the next two years".  I
>> don't think it'd go over well.
>
> "Two years" or anyway "indistinguishable from sleep" is never really
> acceptable.  Because then emacs can't process key/mouse events
> regardless how many threads you have in the backend.
>
> We need to think in terms of UI timings here.  Reference is the kbd
> repeat rate, say 30ms, which is also in the range of eye perception
> capability and frame repeat rates.  Up to this is smooth.  More can
> be tolerable though, even up to 1 or more seconds.  Say for popping
> up new windows, frames, etc.
>
> So if your elisp doesn't finish in 30ms, you know ;)
>
>>> * break command_loop_1() such that it can be used to handle just one
>>>  event which is to handle scrollbar messages because the widgets
>>>  run their own message loop deep in windows.  Otherwise all the
>>>  scrolling would happen only after you release the mouse button.
>>
>> I doubt that the scrollbar is the only special case we'd need to consider.
>
> AFAICS scrollbar and paint. And maybe clipboard rendering.  Emacs
> handles paint directly via expose_frame() though, not via the event
> queue.
>
>> I don't understand how this approach helps.  The problem, AIUI, isn't
>> that we have Lisp events, but that we read input and wait for processes
>> in many places, and it's hard to be confident that each place we pump
>> messages is a safe place to process lisp code.  I don't understand how
>> draining the lisp event queue would help.
>
> As I see it this is just recursion.   Such things happen all the time
> in all GUIs that use callbacks.  Windows, GTK, whatever.  It happens
> in emacs already:
>
>      (run-with-idle-timer ... lisp-code)
>
> --- grischka
>
>
> diff --git a/nt/config.nt b/nt/config.nt
> index d612a41..dd4cd38 100644
> --- a/nt/config.nt
> +++ b/nt/config.nt
> @@ -467,6 +467,8 @@ extern char *getenv ();
>  #endif
>  #endif
>
> +#define SINGLE_THREAD
> +
>  /* Redefine abort.  */
>  #ifdef HAVE_NTGUI
>  #define abort  w32_abort
> diff --git a/src/keyboard.c b/src/keyboard.c
> index d533213..e824682 100644
> --- a/src/keyboard.c
> +++ b/src/keyboard.c
> @@ -1290,7 +1290,7 @@ cancel_hourglass_unwind (Lisp_Object arg)
>  EXFUN (Fwindow_system, 1);
>
>  Lisp_Object
> -command_loop_1 (void)
> +command_loop_1a (int repeat)
>  {
>   Lisp_Object cmd;
>   Lisp_Object keybuf[30];
> @@ -1336,7 +1336,7 @@ command_loop_1 (void)
>   if (!CONSP (last_command_event))
>     current_kboard->Vlast_repeatable_command = real_this_command;
>
> -  while (1)
> +  while (repeat || detect_input_pending())
>     {
>       if (! FRAME_LIVE_P (XFRAME (selected_frame)))
>        Fkill_emacs (Qnil);
> @@ -1659,6 +1659,12 @@ command_loop_1 (void)
>     }
>  }
>
> +Lisp_Object
> +command_loop_1 ()
> +{
> +    return command_loop_1a(1);
> +}
> +
>  /* Adjust point to a boundary of a region that has such a property
>    that should be treated intangible.  For the moment, we check
>    `composition', `display' and `invisible' properties.
> diff --git a/src/lisp.h b/src/lisp.h
> index cfff42a..efe76f6 100644
> --- a/src/lisp.h
> +++ b/src/lisp.h
> @@ -2013,6 +2013,10 @@ extern char *stack_bottom;
>    This is a good thing to do around a loop that has no side effects
>    and (in particular) cannot call arbitrary Lisp code.  */
>
> +#if defined HAVE_NTGUI && defined SINGLE_THREAD
> +#define ELSE_PENDING_SIGNALS else w32_peek_messages();
> +void w32_peek_messages();
> +#else
>  #ifdef SYNC_INPUT
>  extern void process_pending_signals (void);
>  extern int pending_signals;
> @@ -2022,6 +2026,7 @@ extern int pending_signals;
>  #else  /* not SYNC_INPUT */
>  #define ELSE_PENDING_SIGNALS
>  #endif /* not SYNC_INPUT */
> +#endif /* not HAVE_NTGUI */
>
>  #define QUIT                                           \
>   do {                                                 \
> diff --git a/src/w32fns.c b/src/w32fns.c
> index b09bb0b..93f80f9 100644
> --- a/src/w32fns.c
> +++ b/src/w32fns.c
> @@ -87,6 +87,10 @@ extern const char *const lispy_function_keys[];
>  static unsigned hourglass_timer = 0;
>  static HWND hourglass_hwnd = NULL;
>
> +#ifdef SINGLE_THREAD
> +int w32_single_thread;
> +#endif
> +
>  #ifndef IDC_HAND
>  #define IDC_HAND MAKEINTRESOURCE(32649)
>  #endif
> @@ -2242,17 +2246,15 @@ unregister_hot_keys (HWND hwnd)
>
>  /* Main message dispatch loop. */
>
> -static void
> -w32_msg_pump (deferred_msg * msg_buf)
> +unsigned dispatch_msg (MSG *pmsg)
>  {
>   MSG msg;
>   int result;
>   HWND focus_window;
>
> -  msh_mousewheel = RegisterWindowMessage (MSH_MOUSEWHEEL);
> +  result = 0;
> +  msg = *pmsg;
>
> -  while (GetMessage (&msg, NULL, 0, 0))
> -    {
>       if (msg.hwnd == NULL)
>        {
>          switch (msg.message)
> @@ -2269,8 +2271,10 @@ w32_msg_pump (deferred_msg * msg_buf)
>                  and older versions will never be patched.  */
>               CoInitialize (NULL);
>              w32_createwindow ((struct frame *) msg.wParam);
> +#ifndef SINGLE_THREAD
>              if (!PostThreadMessage (dwMainThreadId, WM_EMACS_DONE, 0, 0))
>                abort ();
> +#endif
>              break;
>            case WM_EMACS_SETLOCALE:
>              SetThreadLocale (msg.wParam);
> @@ -2278,9 +2282,11 @@ w32_msg_pump (deferred_msg * msg_buf)
>              break;
>            case WM_EMACS_SETKEYBOARDLAYOUT:
>              result = (int) ActivateKeyboardLayout ((HKL) msg.wParam, 0);
> +#ifndef SINGLE_THREAD
>              if (!PostThreadMessage (dwMainThreadId, WM_EMACS_DONE,
>                                      result, 0))
>                abort ();
> +#endif
>              break;
>            case WM_EMACS_REGISTER_HOT_KEY:
>              focus_window = GetFocus ();
> @@ -2300,8 +2306,10 @@ w32_msg_pump (deferred_msg * msg_buf)
>                  cell is never made into garbage and is not relocated by
>                  GC.  */
>              XSETCAR ((Lisp_Object) ((EMACS_INT) msg.lParam), Qnil);
> +#ifndef SINGLE_THREAD
>              if (!PostThreadMessage (dwMainThreadId, WM_EMACS_DONE, 0, 0))
>                abort ();
> +#endif
>              break;
>            case WM_EMACS_TOGGLE_LOCK_KEY:
>              {
> @@ -2331,9 +2339,12 @@ w32_msg_pump (deferred_msg * msg_buf)
>                                 KEYEVENTF_EXTENDEDKEY | KEYEVENTF_KEYUP, 0);
>                    cur_state = !cur_state;
>                  }
> +#ifndef SINGLE_THREAD
>                if (!PostThreadMessage (dwMainThreadId, WM_EMACS_DONE,
>                                        cur_state, 0))
>                  abort ();
> +#endif
> +               result = cur_state;
>              }
>              break;
>  #ifdef MSG_DEBUG
> @@ -2349,11 +2360,37 @@ w32_msg_pump (deferred_msg * msg_buf)
>          DispatchMessage (&msg);
>        }
>
> +    return result;
> +}
> +
> +static void
> +w32_msg_pump (deferred_msg * msg_buf)
> +{
> +#ifndef SINGLE_THREAD
> +  MSG msg;
> +  msh_mousewheel = RegisterWindowMessage (MSH_MOUSEWHEEL);
> +  while (GetMessage (&msg, NULL, 0, 0))
> +    {
> +      dispatch_msg(&msg);
>       /* Exit nested loop when our deferred message has completed.  */
>       if (msg_buf->completed)
>        break;
>     }
> +#endif
> +}
> +
> +#ifdef SINGLE_THREAD
> +unsigned do_thread_msg(unsigned threadid, unsigned umsg, WPARAM wParam,
> LPARAM lParam)
> +{
> +    MSG msg;
> +    msg.hwnd = NULL;
> +    msg.message = umsg;
> +    msg.wParam = wParam;
> +    msg.lParam = lParam;
> +    return dispatch_msg(&msg);
>  }
> +#endif
> +
>
>  deferred_msg * deferred_msg_head;
>
> @@ -2427,7 +2464,9 @@ complete_deferred_msg (HWND hwnd, UINT msg, LRESULT
> result)
>   msg_buf->completed = 1;
>
>   /* Ensure input thread is woken so it notices the completion.  */
> +#ifndef SINGLE_THREAD
>   PostThreadMessage (dwWindowsThreadId, WM_NULL, 0, 0);
> +#endif
>  }
>
>  static void
> @@ -2448,7 +2487,9 @@ cancel_all_deferred_msgs (void)
>   /* leave_crit (); */
>
>   /* Ensure input thread is woken so it notices the completion.  */
> +#ifndef SINGLE_THREAD
>   PostThreadMessage (dwWindowsThreadId, WM_NULL, 0, 0);
> +#endif
>  }
>
>  DWORD WINAPI
> @@ -2654,6 +2695,10 @@ w32_wnd_proc (HWND hwnd, UINT msg, WPARAM wParam,
> LPARAM lParam)
>               dedicated to one frame and does not bother checking
>               that hwnd matches before combining them.  */
>             my_post_msg (&wmsg, hwnd, WM_EMACS_PAINT, wParam, lParam);
> +#ifdef SINGLE_THREAD
> +            w32_read_socket(0, 0, NULL);
> +            //command_loop_1a(0);
> +#endif
>
>             return 0;
>           }
> @@ -3205,6 +3250,10 @@ w32_wnd_proc (HWND hwnd, UINT msg, WPARAM wParam,
> LPARAM lParam)
>        {
>          wmsg.dwModifiers = w32_get_modifiers ();
>          my_post_msg (&wmsg, hwnd, msg, wParam, lParam);
> +#ifdef SINGLE_THREAD
> +          if (msg == WM_VSCROLL)
> +            command_loop_1a(0);
> +#endif
>          return 0;
>        }
>
> @@ -3808,11 +3857,15 @@ w32_wnd_proc (HWND hwnd, UINT msg, WPARAM wParam,
> LPARAM lParam)
>  static void
>  my_create_window (struct frame * f)
>  {
> +#ifdef SINGLE_THREAD
> +  do_thread_msg (dwWindowsThreadId, WM_EMACS_CREATEWINDOW, (WPARAM)f, 0);
> +#else
>   MSG msg;
>
>   if (!PostThreadMessage (dwWindowsThreadId, WM_EMACS_CREATEWINDOW,
> (WPARAM)f, 0))
>     abort ();
>   GetMessage (&msg, NULL, WM_EMACS_DONE, WM_EMACS_DONE);
> +#endif
>  }
>
>
> @@ -6308,6 +6361,15 @@ The return value is the hotkey-id if registered,
> otherwise nil.  */)
>
>       /* Notify input thread about new hot-key definition, so that it
>         takes effect without needing to switch focus.  */
> +#ifdef SINGLE_THREAD
> +#ifdef USE_LISP_UNION_TYPE
> +      do_thread_msg (dwWindowsThreadId, WM_EMACS_REGISTER_HOT_KEY,
> +                         (WPARAM) key.i, 0);
> +#else
> +      do_thread_msg (dwWindowsThreadId, WM_EMACS_REGISTER_HOT_KEY,
> +                         (WPARAM) key, 0);
> +#endif
> +#else
>  #ifdef USE_LISP_UNION_TYPE
>       PostThreadMessage (dwWindowsThreadId, WM_EMACS_REGISTER_HOT_KEY,
>                         (WPARAM) key.i, 0);
> @@ -6315,6 +6377,7 @@ The return value is the hotkey-id if registered,
> otherwise nil.  */)
>       PostThreadMessage (dwWindowsThreadId, WM_EMACS_REGISTER_HOT_KEY,
>                         (WPARAM) key, 0);
>  #endif
> +#endif
>     }
>
>   return key;
> @@ -6336,6 +6399,16 @@ DEFUN ("w32-unregister-hot-key",
> Fw32_unregister_hot_key,
>     {
>       /* Notify input thread about hot-key definition being removed, so
>         that it takes effect without needing focus switch.  */
> +#ifdef SINGLE_THREAD
> +#ifdef USE_LISP_UNION_TYPE
> +      do_thread_msg (dwWindowsThreadId, WM_EMACS_UNREGISTER_HOT_KEY,
> +                             (WPARAM) XINT (XCAR (item)), (LPARAM) item.i);
> +#else
> +      do_thread_msg (dwWindowsThreadId, WM_EMACS_UNREGISTER_HOT_KEY,
> +                             (WPARAM) XINT (XCAR (item)), (LPARAM) item);
> +
> +#endif
> +#else
>  #ifdef USE_LISP_UNION_TYPE
>       if (PostThreadMessage (dwWindowsThreadId, WM_EMACS_UNREGISTER_HOT_KEY,
>                             (WPARAM) XINT (XCAR (item)), (LPARAM) item.i))
> @@ -6347,6 +6420,7 @@ DEFUN ("w32-unregister-hot-key",
> Fw32_unregister_hot_key,
>          MSG msg;
>          GetMessage (&msg, NULL, WM_EMACS_DONE, WM_EMACS_DONE);
>        }
> +#endif
>       return Qt;
>     }
>   return Qnil;
> @@ -6401,6 +6475,9 @@ is set to off if the low bit of NEW-STATE is zero,
> otherwise on.  */)
>   (Lisp_Object key, Lisp_Object new_state)
>  {
>   int vk_code;
> +#ifdef SINGLE_THREAD
> +  unsigned ret;
> +#endif
>
>   if (EQ (key, intern ("capslock")))
>     vk_code = VK_CAPITAL;
> @@ -6414,6 +6491,16 @@ is set to off if the low bit of NEW-STATE is zero,
> otherwise on.  */)
>   if (!dwWindowsThreadId)
>     return make_number (w32_console_toggle_lock_key (vk_code, new_state));
>
> +#ifdef SINGLE_THREAD
> +#ifdef USE_LISP_UNION_TYPE
> +  ret = do_thread_msg (dwWindowsThreadId, WM_EMACS_TOGGLE_LOCK_KEY,
> +                         (WPARAM) vk_code, (LPARAM) new_state.i);
> +#else
> +  ret = do_thread_msg (dwWindowsThreadId, WM_EMACS_TOGGLE_LOCK_KEY,
> +                         (WPARAM) vk_code, (LPARAM) new_state);
> +#endif
> +  return make_number (ret);
> +#else
>  #ifdef USE_LISP_UNION_TYPE
>   if (PostThreadMessage (dwWindowsThreadId, WM_EMACS_TOGGLE_LOCK_KEY,
>                         (WPARAM) vk_code, (LPARAM) new_state.i))
> @@ -6427,6 +6514,7 @@ is set to off if the low bit of NEW-STATE is zero,
> otherwise on.  */)
>       return make_number (msg.wParam);
>     }
>   return Qnil;
> +#endif
>  }
>
>  DEFUN ("w32-window-exists-p", Fw32_window_exists_p, Sw32_window_exists_p,
> @@ -7053,6 +7141,14 @@ Set this to nil to get the old behavior for
> repainting; this should
>  only be necessary if the default setting causes problems.  */);
>   w32_strict_painting = 1;
>
> +#ifdef SINGLE_THREAD
> +  DEFVAR_BOOL ("w32-single-thread",
> +              &w32_single_thread,
> +              doc: /* using single thread */);
> +
> +  w32_single_thread = 1;
> +#endif
> +
>  #if 0 /* TODO: Port to W32 */
>   defsubr (&Sx_change_window_property);
>   defsubr (&Sx_delete_window_property);
> diff --git a/src/w32proc.c b/src/w32proc.c
> index 1c009c7..49eab27 100644
> --- a/src/w32proc.c
> +++ b/src/w32proc.c
> @@ -2028,7 +2028,11 @@ If successful, the new locale id is returned,
> otherwise nil.  */)
>   /* Need to set input thread locale if present.  */
>   if (dwWindowsThreadId)
>     /* Reply is not needed.  */
> +#ifdef SINGLE_THREAD
> +    do_thread_msg (dwWindowsThreadId, WM_EMACS_SETLOCALE, XINT (lcid), 0);
> +#else
>     PostThreadMessage (dwWindowsThreadId, WM_EMACS_SETLOCALE, XINT (lcid),
> 0);
> +#endif
>
>   return make_number (GetThreadLocale ());
>  }
> @@ -2194,6 +2198,10 @@ If successful, the new layout id is returned,
> otherwise nil.  */)
>   /* Synchronize layout with input thread.  */
>   if (dwWindowsThreadId)
>     {
> +#ifdef SINGLE_THREAD
> +      if (0 == do_thread_msg (dwWindowsThreadId,
> WM_EMACS_SETKEYBOARDLAYOUT, (WPARAM) kl, 0))
> +        return Qnil;
> +#else
>       if (PostThreadMessage (dwWindowsThreadId, WM_EMACS_SETKEYBOARDLAYOUT,
>                             (WPARAM) kl, 0))
>        {
> @@ -2203,6 +2211,7 @@ If successful, the new layout id is returned,
> otherwise nil.  */)
>          if (msg.wParam == 0)
>            return Qnil;
>        }
> +#endif
>     }
>   else if (!ActivateKeyboardLayout ((HKL) kl, 0))
>     return Qnil;
> diff --git a/src/w32term.c b/src/w32term.c
> index cd4ee54..83eed4a 100644
> --- a/src/w32term.c
> +++ b/src/w32term.c
> @@ -4018,7 +4018,7 @@ static char dbcs_lead = 0;
>    recursively with different messages by the system.
>  */
>
> -static int
> +int
>  w32_read_socket (struct terminal *terminal, int expected,
>                 struct input_event *hold_quit)
>  {
> @@ -6294,6 +6294,12 @@ w32_initialize (void)
>                   GetCurrentProcess (), &hMainThread, 0, TRUE,
> DUPLICATE_SAME_ACCESS);
>
>   /* Wait for thread to start */
> +#ifdef SINGLE_THREAD
> +  DuplicateHandle (GetCurrentProcess (), GetCurrentThread (),
> +                   GetCurrentProcess (), &hWindowsThread, 0, TRUE,
> DUPLICATE_SAME_ACCESS);
> +  dwWindowsThreadId = GetCurrentThreadId ();
> +  msh_mousewheel = RegisterWindowMessage (MSH_MOUSEWHEEL);
> +#else
>   {
>     MSG msg;
>
> @@ -6305,6 +6311,7 @@ w32_initialize (void)
>
>     GetMessage (&msg, NULL, WM_EMACS_DONE, WM_EMACS_DONE);
>   }
> +#endif
>
>   /* It is desirable that mainThread should have the same notion of
>      focus window and active window as windowsThread.  Unfortunately, the
> diff --git a/src/w32xfns.c b/src/w32xfns.c
> index df9acca..a3c1af4 100644
> --- a/src/w32xfns.c
> +++ b/src/w32xfns.c
> @@ -299,11 +299,19 @@ drain_message_queue (void)
>   MSG msg;
>   while (PeekMessage (&msg, NULL, 0, 0, PM_REMOVE))
>     {
> +#ifdef SINGLE_THREAD
> +      dispatch_msg(&msg);
> +#else
>       TranslateMessage (&msg);
>       DispatchMessage (&msg);
> +#endif
>     }
>  }
>
> +void w32_peek_messages()
> +{
> +    drain_message_queue ();
> +}
>
>  /*
>  *    XParseGeometry parses strings of the form
>
>