This is the mail archive of the cygwin-patches mailing list for the Cygwin project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
Hi Takashi, On Sep 3 12:05, Takashi Yano wrote: > > > - Sleep (60); /* Wait for pty_master_fwd_thread() */ > > > + Sleep (20); /* Wait for pty_master_fwd_thread() */ > > > > Isn't that a separate issue as well? A separate patch may be in order > > here, kind of like "Cygwin: pseudo console: reduce time sleeping ..." > > with a short description why that makes sense? > > Actually it is not. The wait time became able to be reduced by > redesigning switching of r/w pipes which managed via variable > switch_to_pcon. So I think this should be included in this patch. Ok. > > However, I have a few questions in terms of the code in general, namely > > in terms of > > > > ALWAYS_USE_PCON > > USE_API_HOOK > > USE_OWN_NLS_FUNC > > > > Can you describe again why you introduced these macros? > > These are defined for debugging purpose. > > If ALWAYS_USE_PCON is defined to true, pseudo console pipe is used for > all process including pure cygwin process. Usually, this should be false > so that the cygwin process use named pipe as previous. > > USE_API_HOOK is for enabling/disabling the API hook to detect direct > console access in cygwin process. This should be true so that the > r/w pipe switching is set to pseudo console side for the cygwin > process which directly access console. > > As for USE_OWN_NLS_FUNC, I have not decided yet which codes should be > used. If USE_OWN_NLS_FUNC is false, setlocale (LC_CTYPE, "") is > called therefore it may affect to some programs wihch do not call > setlocale(). > > > In terms of USE_API_HOOK: > > > > - Shouldn't the hook_api function be moved to hookapi.cc? > > I will move it into hookapi.cc, and post it as a separate patch. > > > - Do we really want to hook every invocation of WriteFile/ReadFile? > > Doesn't that potentially slow down an exec'ed process a lot? > > We're still not using the NT functions throughout outside of the > > console/tty code. > > I measured the time for calling WriteFile() 1000000 times writing > 1 byte to a disk file for each call. Files are not affected in Cygwin, fortunately. They use NtReadFile/NtWriteFile, not ReadFile/WriteFile. However, other stuff is still affected... > Not hooked: > Total: 4.558321 seconsd > > Hooked: > Total: 6.522692 seconsd > > Hooking causes slow down indeed. It seems that GetConsoleMode() > is slow. So I have added the check for GetFileType() before > GetConsoleMode() and made check in two stages. > > Hooked (new): > Total: 5.217996 seconsd > > This results in speed up a little. I will post another patch for this. This is a slowdown of about 15%. That's quite a lot. Can't you just check the incoming handle against the interesting handles somehow? If there's no other way around it, we should at least make sure (in a separate patch) that Cygwin calls NtReadFile/NtWriteFile throughout, except in tty and console code. > > In terms of USE_OWN_NLS_FUNC: > > > > - Why do we need this function at all? Can't this be handled by > > __loadlocale instead? If not, what is __loadlocale missing to make > > this work without duplicating the function? > > Calling __loadlocale() here causes execution error. > > mintty: > 0 [main] tcsh 1901 sig_send: error sending signal 6, pid 1901, pipe handle 0x0, nb 0, packsize 164, Win32 error 6 > > script: > Script started, file is typescript > script: failed to execute /bin/tcsh: Bad address > Script done, file is typescript > > I could not find out the reason. Some kind of initialization which > is needed by __loadlocale() may not be done yet. So I copied > only necessary part from __loadlocale() and nl_langinfo(). > > Simply, > path_conv a ("/usr/share/locale/locale.alias"); > also causes errors on starting mintty. > > Ideally, the cause of the error should be found out, I suppose. Indeed. We can keep the code in for now, but the end result should call a tweaked version of __loadlocale instead. As long as the tweak only requires a single extra argument, or if the category or new_locale argument can be used as indicator to trigger the required special behavour, we should have no problem to get that into newlib. Thanks, Corinna -- Corinna Vinschen Cygwin Maintainer
Attachment:
signature.asc
Description: PGP signature
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |