On Tue, 21 Jan 2025 14:45:10 +0100 Corinna Vinschen wrote: > Hi Takashi, > > There's a minor style thingy and one question... > > On Jan 21 12:15, Takashi Yano wrote: > > +++ b/winsup/cygwin/sigproc.cc > > @@ -742,6 +742,12 @@ sig_send (_pinfo *p, siginfo_t& si, _cygtls *tls) > > memcpy (p, si._si_commune._si_str, n); p += n; > > } > > > > + unsigned cw_mask; > > + if (pack.si.si_signo == __SIGFLUSHFAST) > > + cw_mask = 0; > > + else > > + cw_mask = cw_sig_restart; > > + > > This could be a one-liner, unsigned cw_mask = ... ? ... : ...; > > > DWORD nb; > > BOOL res; > > /* Try multiple times to send if packsize != nb since that probably > > @@ -751,8 +757,7 @@ sig_send (_pinfo *p, siginfo_t& si, _cygtls *tls) > > res = WriteFile (sendsig, leader, packsize, &nb, NULL); > > if (!res || packsize == nb) > > break; > > - if (cygwait (NULL, 10, cw_sig_eintr) == WAIT_SIGNALED) > > - _my_tls.call_signal_handler (); > > + cygwait (NULL, 10, cw_mask); > > This bugs me a bit. While your solution nicely wraps the entire > timer problem into cygwait(), the downside is that each invocation > of cygwait() creates its own timer. Theoretically, given this is in a > loop with up to 100 iterations, you have up to 100 additional timer > create/destroy sequences. > > So the question is, do you think this matters at all in this scenario, > given we're in a 10 ms wait state anyway? > > If you think that's not an issue, feel free to apply the patch with > just the one-liner above.
Thansk for reviewing. cygwait (NULL, 10, cw_mask) is just waiting for resolving pipe full. Therefore, I think the overhead of creating and destroying a timer every 10 msec in the wait loop is small enough to be negligible. That is, the CPU load will be almost the same if we avoid it. BTW, I'm happy if you could review also: [PATCH v2] Cygwin: signal: Avoid frequent TLS lock/unlock for SIGCONT processing Thanks! -- Takashi Yano <takashi.y...@nifty.ne.jp>
