tikv / jemallocator

Rust allocator using jemalloc as a backend
Other
364 stars 62 forks source link

Fix windows support #99

Open roblabla opened 3 months ago

roblabla commented 3 months ago

The windows targets weren't working at all for me:

  1. The wrong target was being used. i686-pc-win32 doesn't exist as far as jemalloc's build system is concerned, only i686-pc-mingw32 does (and if jemalloc detects the compiler is MSVC, it will automatically switch to an MSVC-based build).
  2. The library wasn't getting found, because it was called jemalloc_s instead of just jemalloc
  3. The unprefixed_on_supported_target feature was broken because jemalloc automatically prefixes macos and windows targets for some reason.

Finally, I also added support for the win7 target, which is a tier3 target supporting windows 7.

Note that this is tested manually on my end using a bit of a bespoke cross-compilation toolchain. The crate doesn't successfully build on a more "normal" toolchain due to autoconf failing to build when the compiler path contains spaces. I'd like to add a CI, but I'd need to first need to find a way to fix that issue...

BusyJay commented 3 months ago

Interesting, you can also update the action files to add windows target and prove it works.

roblabla commented 3 months ago

So as I said, I have a bit of a weird setup where I use a cross-compiling toolchain to build the crate, and that works.

However, when I tried to add it in CI, but doing the naive thing of just running cargo test in a windows runner doesn't work, because it tries to run with CC=C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.40.33807\bin\HostX64\x64\cl.exe, and that doesn't work because of the space in the "Program Files". See https://github.com/JustRustThings/jemallocator/actions/runs/10164835677/job/28111216397

This might be fixable by entering a visual studio shell so the cl.exe program is in the PATH. I'll give it a try.

BusyJay commented 3 months ago

I don't think space in path is the problem.

checking for x86_64-pc-mingw32-gcc... C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.40.33807\bin\HostX64\x64\cl.exe

The line shows that full compiler path is used. However the target it tries doesn't match the compiler it uses.

roblabla commented 3 months ago

The space is absolutely the problem, but the logs don't show it clearly. If I run the build locally on a windows vm and check the config.log, it has an error to the tune of "C:\Program: no such file or directory", which stops the build.

Autoconf does not support spaces in the compiler path. It's a rather well-known problem. I just had a pretty solid idea to avoid it though: I can use the short path syntax (c:\progra~2) to avoid the space :eyes:.

The target is not wrong. As I explain in the post, the way you do an msvc build with jemalloc is by building for mingw/cygwin with an msvc compiler in CC. The autoconf script will detect that and set use_msvc to yes, at which point the msvc codepaths will be used.

roblabla commented 1 month ago

Finally found the time to push this across the finish line, this PR now builds correctly in CI: https://github.com/roblabla/jemallocator/actions/runs/10995602458

Had to fix a compilation issue in jemalloc when using MSVC 2022, which I submitted both:

I then had to update the submodule to point to the new commit, and regenerate the vendored configure script.

With all this in place, windows support is now working properly.

I also cleaned up the commit a little, removing the DBG commits and providing more description to some otherwise "bare" commits.

Aerocatia commented 1 week ago

Hi, I tested this branch and it does not work when targeting windows-gnu from a Linux host. It does not work on the upstream version either. It complains about finding the static library and background_threads_runtime_support.

Aerocatia commented 1 week ago

Giving it a quick look I would say the issue is build.rs assuming the target for itself is the actual build target.