[PATCH1/2] Use spawn() on Cygwin
Hello! Please take this patch, Cygwin team told that they would like to integrate with upstream. I have already posted it some time ago but got no reply. The patch significantly improves performance of Make under Cygwin. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia make-3.82.90-1-use-spawn-on-cygwin.diff Description: Binary data ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
[PATCH 2/2] Do not use DOS paths on Cygwin
Currently make's configure suggests that it should use DOS-style paths on Cygwin. This is not true, and this assumption makes path-related mechanisms to work incorrectly. Currently Cygwin package supplies manual hint in config.cache in order to work around this. I think we should also ask MinGW team about __MSYS__ definition. MSys is close to Cygwin, so perhaps this should also be removed. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia make-3.82.90-1-no-dos-paths-on-cygwin.diff Description: Binary data ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
Re: [PATCH 2/2] Do not use DOS paths on Cygwin
> From: Pavel Fedin > Date: Tue, 30 Jul 2013 14:44:58 +0400 > > Currently make's configure suggests that it should use DOS-style paths on > Cygwin. This is not true, and this assumption makes path-related mechanisms > to work incorrectly. Currently Cygwin package supplies manual hint in > config.cache in order to work around this. Sorry, I don't understand the problem you are trying to solve. Could you please explain it in more detail? This feature was added in response to an explicit request of some Cygwin users. I don't want to remove it without a very good reason. After all, if you don't need DOS-style file names, you can simply refrain from using them in your Makefiles, and then you will not see any effect of this support. > I think we should also ask MinGW team about __MSYS__ definition. MSys is > close to Cygwin, so perhaps this should also be removed. MSYS supports DOS-style file names, so removing that would seem wrong. However, I don't speak for MSYS developers, so I'll defer to their judgment. Thanks. ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
Re: [PATCH1/2] Use spawn() on Cygwin
> From: Pavel Fedin > Date: Tue, 30 Jul 2013 14:42:23 +0400 > > Please take this patch, Cygwin team told that they would like to integrate > with upstream. I have already posted it some time ago but got no reply. > The patch significantly improves performance of Make under Cygwin. Thanks. Is there any discussion we could read about that with the details of the problem and how/why does the proposed patch solves it? In general, I feel it's wrong to do this: Cygwin is a Posix platform, so it should be using the Posix code, to be as compatible with other Posix platforms as possible. EMX is not a Posix platform, so using its code will likely make the Cygwin Make deviate from Posix behavior at times. ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
Re: [PATCH1/2] Use spawn() on Cygwin
Hi... bug-make-bounces+roland.schwingel=onevision@gnu.org wrote on 30.07.2013 17:39:10: > > From: Pavel Fedin > > Date: Tue, 30 Jul 2013 14:42:23 +0400 > > > > Please take this patch, Cygwin team told that they would like to integrate > > with upstream. I have already posted it some time ago but got no reply. > > The patch significantly improves performance of Make under Cygwin. > > Thanks. > > Is there any discussion we could read about that with the details of > the problem and how/why does the proposed patch solves it? > > In general, I feel it's wrong to do this: Cygwin is a Posix platform, > so it should be using the Posix code, to be as compatible with other > Posix platforms as possible. EMX is not a Posix platform, so using > its code will likely make the Cygwin Make deviate from Posix behavior > at times. I am using Pavels patch for some months now in my private version of gnumake on cygwin heavily and I could not find any regression with it up to now and gnumake is in my use cases clearly faster. It is still not a stallion badged red italian car, but it is cruising its curves noticably snappier on cygwin now. Using EMX is a shortcut to achieve the usage of spawn() over fork(). It might not be the cleanest choice but it works. Maybe Pavel finds the time to make the patch distinct to cygwin itself. Just my $0.02, Roland___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
Re: [PATCH 2/2] Do not use DOS paths on Cygwin
Hi... bug-make-bounces+roland.schwingel=onevision@gnu.org wrote on 30.07.2013 17:43:10: > > Currently make's configure suggests that it should use DOS-style paths on > > Cygwin. This is not true, and this assumption makes path-related mechanisms > > to work incorrectly. Currently Cygwin package supplies manual hint in > > config.cache in order to work around this. > > Sorry, I don't understand the problem you are trying to solve. Could > you please explain it in more detail? > > This feature was added in response to an explicit request of some > Cygwin users. I don't want to remove it without a very good reason. > After all, if you don't need DOS-style file names, you can simply > refrain from using them in your Makefiles, and then you will not see > any effect of this support. > > > I think we should also ask MinGW team about __MSYS__ definition. MSys is > > close to Cygwin, so perhaps this should also be removed. > > MSYS supports DOS-style file names, so removing that would seem wrong. > However, I don't speak for MSYS developers, so I'll defer to their > judgment. I want to throw in my $0.02 here, too. But this time in the other direction. Sorry Pavel. I clearly think the DOS paths mode should remain in even for cygwin. I know that there are objections in cygwins top level management against this. Cygwin wants to have DOS path mode out as - as far as I understand it. Therefore I am maintaining my private version of gnumake for many years now with enabled DOS path mode (and some little other things). Also I could not see any problems building packages on cygwin for cywin with my version of gnumake, but I have to confess that is not my regular use case. I believe I am using a DOS paths enabled gnumake on cygwin now for over 10 years. Having enabled DOS pathes in gnumake makes life with non cygwin tools (eg. javac and other things) in makefiles a lot easier. Again this is my personal opinion. For MSYS I believe it could not be ripped out. It is IMHO vital there and would break it. Roland ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
Re: [PATCH1/2] Use spawn() on Cygwin
> Cc: bug-make@gnu.org, Pavel Fedin > From: Roland Schwingel > Date: Tue, 30 Jul 2013 18:12:55 +0200 > > I am using Pavels patch for some months now in my private version of > gnumake on cygwin heavily and I could not find any regression with it up > to now and gnumake is in my use cases clearly faster. Thanks for the info. What version of Make are you using? If it is not the version from the git repo, please try that, and in particular the new --output-sync feature. ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
Re: [PATCH1/2] Use spawn() on Cygwin
On Tue, 2013-07-30 at 18:39 +0300, Eli Zaretskii wrote: > In general, I feel it's wrong to do this: Cygwin is a Posix platform, > so it should be using the Posix code, to be as compatible with other > Posix platforms as possible. EMX is not a Posix platform, so using > its code will likely make the Cygwin Make deviate from Posix behavior > at times. If we decide to take this change I think we should reduce it to a single #define, such as HAVE_W32_SPAWN or similar, as we did with the HAVE_DOS_PATHS, set it in one place based on __EMX__ etc., then use that single macro where we want to check for spawn() in the code. ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
Re: [PATCH1/2] Use spawn() on Cygwin
Hi Eli... Eli Zaretskii wrote on 30.07.2013 18:29:53: > From: Eli Zaretskii > To: Roland Schwingel > Cc: bug-make@gnu.org, p.fe...@samsung.com > Date: 30.07.2013 18:32 > Subject: Re: [PATCH1/2] Use spawn() on Cygwin > > > Cc: bug-make@gnu.org, Pavel Fedin > > From: Roland Schwingel > > Date: Tue, 30 Jul 2013 18:12:55 +0200 > > > > I am using Pavels patch for some months now in my private version of > > gnumake on cygwin heavily and I could not find any regression with it up > > to now and gnumake is in my use cases clearly faster. > > Thanks for the info. > > What version of Make are you using? If it is not the version from the > git repo, please try that, and in particular the new --output-sync > feature. My version is from git, but already some months old (I checked it out on 20130114). So it is not fresh bananas. Currently I am enjailed by a very tight schedule. Maybe I can update my version of gnumake beginning of next week. Sorry, Roland ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
Re: [PATCH 2/2] Do not use DOS paths on Cygwin
> Cc: bug-make@gnu.org, Pavel Fedin > From: Roland Schwingel > Date: Tue, 30 Jul 2013 18:29:07 +0200 > > I clearly think the DOS paths mode should remain in even for cygwin. I > know that there are objections in cygwins top level management against > this. When this feature was introduced for Cygwin Make, the Cygwin maintainer didn't oppose that. ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
Re: [PATCH1/2] Use spawn() on Cygwin
Hi... bug-make-bounces+roland.schwingel=onevision@gnu.org wrote on 30.07.2013 18:37:35: > On Tue, 2013-07-30 at 18:39 +0300, Eli Zaretskii wrote: > > In general, I feel it's wrong to do this: Cygwin is a Posix platform, > > so it should be using the Posix code, to be as compatible with other > > Posix platforms as possible. EMX is not a Posix platform, so using > > its code will likely make the Cygwin Make deviate from Posix behavior > > at times. > > If we decide to take this change I think we should reduce it to a single > #define, such as HAVE_W32_SPAWN or similar, as we did with the > HAVE_DOS_PATHS, set it in one place based on __EMX__ etc., then use that > single macro where we want to check for spawn() in the code. This sounds IMHO reasonable. Roland ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
Re: [PATCH1/2] Use spawn() on Cygwin
On Tue, Jul 30, 2013 at 10:39 AM, Eli Zaretskii wrote: >> From: Pavel Fedin >> Date: Tue, 30 Jul 2013 14:42:23 +0400 >> >> Please take this patch, Cygwin team told that they would like to integrate >> with upstream. I have already posted it some time ago but got no reply. >> The patch significantly improves performance of Make under Cygwin. > > Thanks. > > Is there any discussion we could read about that with the details of > the problem and how/why does the proposed patch solves it? fork() is a very expensive operation in cygwin. we are seeing that in LibreOffice build a lot. I will try to pick up that patch and report numbers on it (we have a pretty big make, with 125K+ targets http://skyfromme.wordpress.com/2013/02/28/one/ ) > > In general, I feel it's wrong to do this: Cygwin is a Posix platform, > so it should be using the Posix code, to be as compatible with other > Posix platforms as possible. EMX is not a Posix platform, so using > its code will likely make the Cygwin Make deviate from Posix behavior > at times. in theory you are right... in practice Cygwin maybe be posix, but it's underlying OS is not and the emulation layer can be very costly. Norbert ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
Re: [PATCH1/2] Use spawn() on Cygwin
> Date: Tue, 30 Jul 2013 11:52:58 -0500 > From: Norbert Thiebaud > Cc: Pavel Fedin , bug-make@gnu.org > > fork() is a very expensive operation in cygwin. Yes, I know. But without it, some things that are expected of a Posix behavior will not work. A notable example is that the child process initially has all the file descriptors and global variables that the parent had. 'spawn' does not necessarily guarantee that. ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
Re: [PATCH1/2] Use spawn() on Cygwin
Dear Eli Zaretskii! On Tue, 30 Jul 2013 18:39:10 +0300 you wrote: > Is there any discussion we could read about that with the details of > the problem and how/why does the proposed patch solves it? The goal of this is exactly what is stated - improve performance. fork() is extremely slow operation on Cygwin because of Windows' nature. Make already had all the code so it took quite a short time to adapt it. I know that gcc toolchain also uses this for speedup. There was a short discussion in Cygwin ML with test results, sorry, i cannot find the URL, Google fails to find it. -- Kind regards, Pavel Fedin ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
Re: [PATCH 2/2] Do not use DOS paths on Cygwin
Dear Eli Zaretskii! On Tue, 30 Jul 2013 18:43:10 +0300 you wrote: > Sorry, I don't understand the problem you are trying to solve. Could > you please explain it in more detail? The actual problem with this is that Make supports either DOS or POSIX style paths, but not simultaneously. I came to this when i developed my previous patch (spawn instead of fork). I needed to rebuild Make, so i typed configure then make. For verification i used 'make test'. All tests which use relative>absolute path conversion failed after this. It took me some time fo figure out what was wrong. Make failed to recognize paths starting from '/' as absolute and corrupted them. It expected absolute paths to have form of 'x:/'. Actually i asked on the Cygwin ML, and they told that they just force this option to off via config.cache. If someone really wants it, then maybe perhaps something like --enable-dos-paths should be introduced ? If the user really wants DOS paths under Cygwin, and completely understands what he is doing, then let him specify this on the command line. -- Kind regards, Pavel Fedin ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
Re: [PATCH 2/2] Do not use DOS paths on Cygwin
> Date: Wed, 31 Jul 2013 01:31:56 +0400 > From: Pavel Fedin > Cc: bug-make@gnu.org > > The actual problem with this is that Make supports either DOS or POSIX > style paths, but not simultaneously. That's not true, at least that's not how things are designed to work. Please show a small self-contained Makefile that could be used to reproduce this problem, and please describe how file names of "the other" style fail with that Makefile. > If someone really wants it, then maybe perhaps something like > --enable-dos-paths should be introduced ? The way things are supposed to work, there's no need for this option. ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
Re: [PATCH1/2] Use spawn() on Cygwin
> Date: Wed, 31 Jul 2013 01:24:45 +0400 > From: Pavel Fedin > Cc: bug-make@gnu.org > > There was a short discussion in Cygwin ML with test results, sorry, i > cannot find the URL, Google fails to find it. Can you at least tell when (year and month) this discussion took place? In any case, I think such a change can only be accepted as an optional feature, preferably controlled by a command-line option. I think it's wrong to use this by default, certainly not to let users a fire escape in case this causes differences in behavior in some use cases. ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
RE: [PATCH1/2] Use spawn() on Cygwin
Hello! Ø Using EMX is a shortcut to achieve the usage of spawn() over fork(). It might not be the cleanest choice but it works. Maybe Pavel finds the time to make the patch distinct to cygwin itself. What do you mean exactly by this ? Unfortunately i really dont have much time to work on this. Additionally makes code IMHO is a horrible mess full of #ifdefs, if we talk about this, then i would say it needs complete refactoring. But its unlikely that this will ever be done because make supports lots of old platforms, many of which almost fell out of use, and nobody will be able to test them all in a reasonable time frame. Unless you (Make developers) make a decision to drop these platforms completely. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
RE: [PATCH1/2] Use spawn() on Cygwin
Hello! > Can you at least tell when (year and month) this discussion took place? I was able to find only this in ML archive: http://cygwin.com/ml/cygwin/2013-01/msg00113.html The rest of discussion was held in private email. For some weird reason patch mails were rejected as spam by Cygwin ML, so i was unable to post there. IIRC i gave them to Reni Urban. Anyway, by the moment they are well tested. I use patched Make for my daily activity, it works fine. > In any case, I think such a change can only be accepted as an optional > feature, preferably controlled by a command-line option. I think it's > wrong to use this by default, certainly not to let users a fire escape > in case this causes differences in behavior in some use cases. Well, i guess you are Masters here and you can do whatever you want. Just my last opinion on this: it's not needed. It would just introduce one more uncertainty and some more conditionals. Both spawn() and fork() work correctly under Cygwin, this is well known fact. The performance issue with fork() by itself is also well known. AFAIK Cygwin maintainers even tried to contact Microsoft in order to resolve this, and Microsoft (of course) apparently doesn't care. I have tested my patched version with make's bundled test suite. As well as by some heavy load like building ARM-targeted GNU toolchain with -j9 option or building Linux kernel (yes, under Cygwin). Works fine. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
RE: [PATCH 2/2] Do not use DOS paths on Cygwin
Hello! The problem is revealed by make's own test suite. The following is test results with DOS paths disabled: --- cut --- $ ./run_make_tests -- Running tests for GNU make on CYGWIN_NT-6.1 fedinw7x64 1.7.22(0.268/5/3) x86_64 GNU Make 3.82.90 -- Finding tests... features/archives ... ok (7 passed) features/comments ... ok (1 passed) features/conditionals ... ok (4 passed) features/default_names .. ok (2 passed) features/double_colon ... ok (11 passed) features/echoing ok (4 passed) features/errors . ok (2 passed) features/escape . ok (6 passed) features/export . ok (12 passed) features/include ok (10 passed) features/mult_rules . ok (2 passed) features/mult_targets ... ok (2 passed) features/order_only . ok (10 passed) features/override ... ok (4 passed) features/parallelism ok (9 passed) features/patspecific_vars ... ok (10 passed) features/patternrules ... ok (10 passed) features/quoting ok (1 passed) features/recursion .. ok (2 passed) features/reinvoke ... ok (5 passed) features/se_explicit ok (9 passed) features/se_implicit ok (9 passed) features/se_statpat . ok (4 passed) features/shell_assignment ... ok (4 passed) features/statipattrules . ok (8 passed) features/targetvars . ok (25 passed) features/varnesting . ok (2 passed) features/vpath .. ok (2 passed) features/vpath2 . ok (1 passed) features/vpath3 . ok (1 passed) features/vpathgpath . ok (1 passed) features/vpathplus .. ok (4 passed) functions/abspath ... ok (1 passed) functions/addprefix . ok (1 passed) functions/addsuffix . ok (2 passed) functions/andor . ok (2 passed) functions/basename .. ok (1 passed) functions/call .. ok (3 passed) functions/dir ... ok (1 passed) functions/error . ok (5 passed) functions/eval .. ok (9 passed) functions/filter-out ok (1 passed) functions/findstring ok (1 passed) functions/flavor ok (1 passed) functions/foreach ... ok (4 passed) functions/if ok (1 passed) functions/join .. ok (1 passed) functions/notdir ok (1 passed) functions/origin ok (1 passed) functions/realpath .. Error running make (expected 0; got 512): make -f work/functions/realpath.mk FAILED (0/1 passed) functions/shell . ok (2 passed) functions/sort .. ok (2 passed) functions/strip . ok (2 passed) functions/substitution .. ok (3 passed) functions/suffix ok (1 passed) functions/value . ok (1 passed) functions/warning ... ok (4 passed) functions/wildcard .. ok (6 passed) functions/word .
