Reduce calls to `cygpath` and support MSYS2

[dra27]

This PR is best reviewed commit by commit. On my machine, building OCaml trunk at https://github.com/ocaml/ocaml/commit/3a13fe6443aaac27007031e8affecf7a08342ddd, re-linking runtime/ocamlrun.exe with the command:

./flexlink.opt.exe -I stdlib/flexdll -exe -chain mingw64 -stack 33554432 -link -municode -o runtime/ocamlrun.exe runtime/prims.o runtime/libcamlrun.a -lws2_32 -lversion -l:libpthread.a -lgcc_eh

takes just over 3 seconds to run. While I'd been aware of this apparent slowdown and had a few hunches, I didn't until recently have a chance to drill down into what was actually going on. So I added some profiling and timing code, expecting to come across something hideously complicated in one or other of the coff parsers, or the symbol-tracking data structures or something which might need some actual computer science to fix. Nah! It turns out, 66% of the time is spent calling cygpath which is called 15 times - 14 of them with identical parameters each time. Oops.

I'm using Cygwin 3.4.6. I don't particularly know/care if this slowness is something which has always been the case, but I expect it's not. Regardless, the discovery is that calling cygpath is slow and, as it happens, we can get away with not making 14 identical calls to it! The issue of course arises from perfectly logical code. When searching for a file (say libws2_32.a), flexlink searches through the various include directories passed to it and then through the list of directories parsed from gcc -print-search-dirs. If that search fails, it passes the entire search path to cygpath and then searches in the result. The problem here is twofold. Firstly, we do this for each file, rather than just once for the entire search path. Secondly, as gcc -print-search-dirs returns directories which will always require this (at least using Cygwin's build of mingw-w64), this conversion happens for a any system library. It's in fact slightly worse than that - for example, if for some reason the user has created C:\usr\lib\gcc\x86_64-w64-mingw32\11 then this will in fact be searched before /usr/lib/gcc/x86_64-w64-mingw32/11 is translated to the actual C:/cygwin64/lib/gcc/x86_64-w64-mingw32/11. The effect is that pretty much every library will trigger this behaviour. There are 4 libraries on the command line above and 10 default libraries which are always scanned for mingw-w64 and there we have 14 of 15 calls.

The 15th call is because on startup flexlink immediately checks if cygpath is available by making a "dummy" call to it.

The first two commits in this set are very minor - some improved use of format strings (as long as OCaml 3.11 doesn't disagree) and a logic error in patch_output which removes an incorrect call to cygpath.

The fix is then in two stages in the last two commits. The first commit removes the startup call to cygpath by tweaking the logic used for invoking cygpath in flexlink itself. Where before the decision to use cygpath was made by testing for its presence, this check is hoisted to the first actual call to cygpath - i.e. on the first call, we are simultaneously determining if cygpath is available and using it. The final commit then recognises that, for a native Windows build of flexlink, the paths returned by Cygwin's x86_64-w64-mingw32-gcc -print-search-dirs should immediately be converted using cygpath (unless, for compatibility, the user has somewhat mistakenly passed -nocygpath). These two commits together then reduce 15 calls to cygpath to just one which on my system takes the command above down to just under 1 second, roughly 80% of which is spent calling cygpath, and the linker. In fixing this, it also made sense to fix #97 which adds support for using MSYS2's mingw-w64 compilers instead. When compiling OCaml this way on my machine, the linking command above takes a mere 0.5s - there are no calls to cygpath at all and the linker itself is slightly faster as it's a native executable.

[dra27]

@jonahbeckford - I've tested this building OCaml in MSYS2 on my machine, but is it easy for you to confirm that this version works for Diskuv?

[nojb]

Just to make sure I understand: this only benefits the Mingw64 toolchain, right?

[dra27]

Yes, that's a very good point to add - msvc doesn't ever call cygpath by default so it's unaffected by this.

[jonahbeckford]

Looks good!

Don't think I need to test this with a time-consuming Vagrant regression test. Used https://github.com/diskuv/dkml-compiler/commit/75f417518ac5b67e69681de17d381bc511528365 and with-dkml make local-install to build, and the following to verify:

Z:\source\dkml-compiler\_opam\src-ocaml\runtime>C:\DiskuvOCaml\BuildTools\VC\Auxiliary\Build\vcvars64.bat                  
**********************************************************************                                                     
** Visual Studio 2019 Developer Command Prompt v16.11.3
** Copyright (c) 2021 Microsoft Corporation
**********************************************************************
[vcvarsall.bat] Environment initialized for: 'x64'

Z:\source\dkml-compiler\_opam\src-ocaml\runtime>where.exe cygpath.exe                                                      
INFO: Could not find files for the given pattern(s).                                                                       

Z:\source\dkml-compiler\_opam\src-ocaml\runtime>..\boot\ocamlruns.exe ..\boot\flexlink.byte.exe -v -chain msvc64 -stack 33554432 -exe -o ocamlrun_PR118.exe prims.obj libcamlrun.lib ws2_32.lib version.lib -- /ENTRY:wmainCRTStartup
+ link /nologo  /implib:"C:\Users\beckf\AppData\Local\Temp\dyndll_impliba6e2c1.lib" /out:"ocamlrun_PR118.exe" /subsystem:console "Z:\source\dkml-compiler\_opam\src-ocaml\boot\flexdll_msvc64.obj" "prims.obj" "libcamlrun.lib" "C:\Program Files (x86)\Windows Kits\10\lib\10.0.22000.0\um\x64\ws2_32.lib" "C:\Program Files (x86)\Windows Kits\10\lib\10.0.22000.0\um\x64\version.lib" "C:\Users\beckf\AppData\Local\Temp\dyndll350d20.obj" /EMITVOLATILEMETADATA:NO /base:0x10000 msvcrt.lib "/ENTRY:wmainCRTStartup"

Z:\source\dkml-compiler\_opam\src-ocaml\runtime>..\boot\ocamlruns.exe ..\boot\flexlink.byte.exe -v -v -chain msvc64 -stack 33554432 -exe -o ocamlrun_PR118.exe prims.obj libcamlrun.lib ws2_32.lib version.lib -- /ENTRY:wmainCRTStartup 
** Use cygpath: maybe
** Search path:
C:\DiskuvOCaml\BuildTools\VC\Tools\MSVC\14.29.30133\lib\x64
C:\Program Files (x86)\Windows Kits\NETFXSDK\4.8\lib\um\x64
C:\Program Files (x86)\Windows Kits\10\lib\10.0.22000.0\ucrt\x64
C:\Program Files (x86)\Windows Kits\10\lib\10.0.22000.0\um\x64
C:\DiskuvOCaml\BuildTools\VC\Tools\MSVC\14.29.30133\lib\x64
C:\Program Files (x86)\Windows Kits\NETFXSDK\4.8\lib\um\x64
C:\Program Files (x86)\Windows Kits\10\lib\10.0.22000.0\ucrt\x64
C:\Program Files (x86)\Windows Kits\10\lib\10.0.22000.0\um\x64
** Default libraries:
msvcrt.lib
** open: Z:\source\dkml-compiler\_opam\src-ocaml\boot\flexdll_msvc64.obj
** open: prims.obj
** open: libcamlrun.lib
** open: C:\Program Files (x86)\Windows Kits\10\lib\10.0.22000.0\um\x64\ws2_32.lib
** open: C:\Program Files (x86)\Windows Kits\10\lib\10.0.22000.0\um\x64\version.lib
** open: C:\Program Files (x86)\Windows Kits\10\lib\10.0.22000.0\um\x64\uuid.lib
lib C:\Program Files (x86)\Windows Kits\10\lib\10.0.22000.0\um\x64\uuid.lib import symbol wscUpdateProductSubStatus
lib C:\Program Files (x86)\Windows Kits\10\lib\10.0.22000.0\um\x64\uuid.lib import symbol wscUpdateProductStatus
lib C:\Program Files (x86)\Windows Kits\10\lib\10.0.22000.0\um\x64\uuid.lib import symbol wscUnregisterSecurityProduct
...

[dra27]

Thanks, @jonahbeckford! Are you able to test it with a mingw-w64 compiler as well, though (or is that not relevant for Diskuv?). I'd got the impression from https://github.com/ocaml/flexdll/issues/97#issuecomment-1005380162 that you might be running with a patched flexlink in order to build with MSYS2, but is that not correct?

[jonahbeckford]

Are you able to test it with a mingw-w64 compiler as well, though (or is that not relevant for Diskuv?).

It is not relevant for Diskuv since (because of my company's tiny size!) I only officially support one compiler (msvc64 and related msvc). If you need the extra eyes, I can also check mingw-w64 on Monday.

[dra27]

That's no problem, thanks - I would have been concerned if there was a chance of breaking things, but if users are expected to be on msvc64, that's all good.

I think this is ready to go, then 🤞