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

[grischka]

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