diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 721e5d089..3a37ea047 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -16,6 +16,11 @@ jobs: strategy: matrix: include: + - name: x86_64-pc-windows-msvc + target: x86_64-pc-windows-msvc + nobgt: 1 + no_tests: 1 + tag: windows-latest - name: x86_64-apple-darwin target: x86_64-apple-darwin nobgt: 0 @@ -59,6 +64,7 @@ jobs: with: submodules: true - name: install + shell: bash run: | if [[ "${{ matrix.rust }}" == "nightly" ]]; then rustup default nightly diff --git a/jemalloc-ctl/src/lib.rs b/jemalloc-ctl/src/lib.rs index 487f719fe..40277007c 100644 --- a/jemalloc-ctl/src/lib.rs +++ b/jemalloc-ctl/src/lib.rs @@ -141,7 +141,7 @@ option! { /// # static ALLOC: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc; /// # /// # fn main() { - /// # #[cfg(not(target_os = "macos"))] { + /// # #[cfg(not(any(target_os = "macos", windows)))] { /// # /// use tikv_jemalloc_ctl::background_thread; /// let bg = background_thread::mib().unwrap(); @@ -171,7 +171,7 @@ option! { /// # static ALLOC: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc; /// # /// # fn main() { - /// # #[cfg(not(target_os = "macos"))] { + /// # #[cfg(not(any(target_os = "macos", windows)))] { /// # /// use tikv_jemalloc_ctl::max_background_threads; /// let m = max_background_threads::mib().unwrap(); diff --git a/jemalloc-ctl/src/macros.rs b/jemalloc-ctl/src/macros.rs index ee4bd5ff0..ee9955f0c 100644 --- a/jemalloc-ctl/src/macros.rs +++ b/jemalloc-ctl/src/macros.rs @@ -69,7 +69,13 @@ macro_rules! r { match stringify!($id) { "background_thread" | "max_background_threads" - if cfg!(target_os = "macos") => return, + if cfg!(any(target_os = "macos", windows)) => return, + "lg_prof_interval" | + "lg_prof_sample" | + "prof_final" | + "prof_leak" | + "prof" + if cfg!(windows) => return, _ => (), } @@ -117,7 +123,7 @@ macro_rules! w { match stringify!($id) { "background_thread" | "max_background_threads" - if cfg!(target_os = "macos") => return, + if cfg!(any(target_os = "macos", windows)) => return, _ => (), } @@ -167,7 +173,7 @@ macro_rules! u { match stringify!($id) { "background_thread" | "max_background_threads" - if cfg!(target_os = "macos") => return, + if cfg!(any(target_os = "macos", windows)) => return, _ => (), } diff --git a/jemalloc-ctl/src/profiling.rs b/jemalloc-ctl/src/profiling.rs index 306ab1a8e..1d9e118e2 100644 --- a/jemalloc-ctl/src/profiling.rs +++ b/jemalloc-ctl/src/profiling.rs @@ -24,9 +24,13 @@ option! { /// # static ALLOC: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc; /// # /// # fn main() { + /// # #[cfg(not(windows))] { + /// # /// use tikv_jemalloc_ctl::profiling; /// let lg_prof_interval = profiling::lg_prof_interval::read().unwrap(); /// println!("average interval between memory profile dumps: {}", lg_prof_interval); + /// # + /// # } // #[cfg(..)] /// # } /// ``` mib_docs: /// See [`lg_prof_interval`]. @@ -49,9 +53,13 @@ option! { /// # static ALLOC: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc; /// # /// # fn main() { + /// # #[cfg(not(windows))] { + /// # /// use tikv_jemalloc_ctl::profiling; /// let lg_prof_sample = profiling::lg_prof_sample::read().unwrap(); /// println!("average interval between allocation samples: {}", lg_prof_sample); + /// # + /// # } // #[cfg(..)] /// # } /// ``` mib_docs: /// See [`lg_prof_sample`]. @@ -79,9 +87,13 @@ option! { /// # static ALLOC: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc; /// # /// # fn main() { + /// # #[cfg(not(windows))] { + /// # /// use tikv_jemalloc_ctl::profiling; /// let prof_final = profiling::prof_final::read().unwrap(); /// println!("dump final memory usage to file: {}", prof_final); + /// # + /// # } // #[cfg(..)] /// # } /// ``` mib_docs: /// See [`prof_final`]. @@ -116,9 +128,13 @@ option! { /// # static ALLOC: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc; /// # /// # fn main() { + /// # #[cfg(not(windows))] { + /// # /// use tikv_jemalloc_ctl::profiling; /// let prof = profiling::prof::read().unwrap(); /// println!("is memory profiling enabled: {}", prof); + /// # + /// # } // #[cfg(..)] /// # } /// ``` mib_docs: /// See [`prof`]. @@ -146,9 +162,13 @@ option! { /// # static ALLOC: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc; /// # /// # fn main() { + /// # #[cfg(not(windows))] { + /// # /// use tikv_jemalloc_ctl::profiling; /// let prof_leak = profiling::prof_leak::read().unwrap(); /// println!("is leak reporting enabled: {}", prof_leak); + /// # + /// # } // #[cfg(..)] /// # } /// ``` mib_docs: /// See [`prof_leak`]. diff --git a/jemalloc-sys/build.rs b/jemalloc-sys/build.rs index b91d3e3d6..7f954a25d 100644 --- a/jemalloc-sys/build.rs +++ b/jemalloc-sys/build.rs @@ -14,8 +14,11 @@ use std::{ fs, io, path::{Path, PathBuf}, process::Command, + sync::OnceLock, }; +use cc::Tool; + include!("src/env.rs"); macro_rules! info { @@ -73,6 +76,116 @@ fn expect_env(name: &str) -> String { env::var(name).unwrap_or_else(|_| panic!("{} was not set", name)) } +/// Attempts to turn the given path into a short path. Returns none in case of +/// error. +fn to_short_path(orig_path: &Path) -> Option { + let out_dir = PathBuf::from(env::var_os("OUT_DIR").expect("OUT_DIR was not set")); + let script_path = out_dir.join("long_to_short.bat"); + std::fs::write(&script_path, "@echo %~s1").unwrap(); + let path = match Command::new(script_path).arg(orig_path).output() { + Ok(v) if v.status.success() => v.stdout, + Ok(v) => { + warning!( + "Failed to expand {} to a short path, the command exited with status {}\nstdout: {}\nstderr: {}", + orig_path.display(), + v.status, + String::from_utf8_lossy(&v.stdout), + String::from_utf8_lossy(&v.stderr), + ); + return None; + } + Err(err) => { + warning!( + "Failed to expand {} to a short path: {}", + orig_path.display(), + err + ); + return None; + } + }; + + // Note: This will only work if the path is valid UTF8. In practice, this + // will always be the case (in fact, we're mostly interested in turning + // "C:\Program Files" into C:\PROG~1). + // + // Should we ever need non-utf8 support here, we'd need to replace the above + // script with a call to GetShortPathNameW, transforming orig_path into + // utf16 through OsStr::encode_wide and the resulting path back into an + // OsString through OsString::from_wide. + let path = match String::from_utf8(path) { + Ok(v) => v, + Err(_err) => { + warning!("Short path of {} is not valid utf-8", orig_path.display()); + return None; + } + }; + + Some(PathBuf::from(path.trim())) +} + +fn find_shell_arg(host: &str, build_dir: &Path) -> Option { + if !cfg!(windows) { + return None; + } + + let make = make_cmd_program(host); + + // Override SHELL to use short paths if it's set to a path using spaces. + // This is often the case if using git bash as a shell, since it will be + // found in C:\Program Files\Git\bin\bash.exe. Github Actions does that. + // + // First, we have to find the SHELL variable. This is often hardcoded + // into the make binary, so we have to use `make -p` to find its value. + let make_vars = Command::new(make) + .current_dir(build_dir) + .arg("-p") + // this stinks. + .arg("-fNUL") + .output(); + match make_vars { + Ok(make_vars_out) + if make_vars_out.status.success() || make_vars_out.status.code() == Some(2) => + { + let out = String::from_utf8_lossy(&make_vars_out.stdout); + let mut found_shell = false; + for line in out.lines() { + if line.starts_with("SHELL ") { + found_shell = true; + // Once we've found the value, we must turn it into a + // short_path if it contains a space. + if let Some(shell) = line.split("= ").nth(1) { + let shell = shell.trim(); + if shell.contains(' ') { + if let Some(short_path) = to_short_path(Path::new(shell)) { + // And finally, we override it using SHELL=X arg + // syntax of makefiles. + return Some(format!("SHELL={}", short_path.display())); + } + } + } + } + } + if !found_shell { + warning!("Did not find SHELL value in make database.",); + } + None + } + Ok(make_vars_out) => { + warning!( + "Failed to run {} -p -fNUL, exited with status {}\nStderr: {}", + make, + make_vars_out.status, + String::from_utf8_lossy(&make_vars_out.stderr) + ); + None + } + Err(err) => { + warning!("Failed to print builtin make values: {:?}", err); + None + } + } +} + // TODO: split main functions and remove following allow. #[allow(clippy::cognitive_complexity)] fn main() { @@ -154,7 +267,19 @@ fn main() { .map(|s| s.to_str().unwrap()) .collect::>() .join(" "); - info!("CC={:?}", compiler.path()); + let mut compiler_path = compiler.path().to_owned(); + if cfg!(windows) && compiler_path.to_string_lossy().contains(' ') { + // Autoconf does not support spaces in the compiler path. This is not + // usually a problem, except on windows where the MSVC compiler (cl.exe) + // is usually found in C:\Program Files, which has a space. Fortunately, + // windows has a trick that can be used: the path can be turned into a + // "short path", turning C:\Program Files into C:\PROGRA~2. Through this + // trick, we can attempt to eliminate the spaces. + if let Some(short_path) = to_short_path(&compiler_path) { + compiler_path = short_path; + } + } + info!("CC={:?}", compiler_path); info!("CFLAGS={:?}", cflags); assert!(out_dir.exists(), "OUT_DIR does not exist"); @@ -189,7 +314,7 @@ fn main() { .replace('\\', "/"), ) .current_dir(&build_dir) - .env("CC", compiler.path()) + .env("CC", compiler_path) .env("CFLAGS", cflags.clone()) .env("LDFLAGS", cflags.clone()) .env("CPPFLAGS", cflags) @@ -198,6 +323,13 @@ fn main() { .arg("--enable-doc=no") .arg("--enable-shared=no"); + for (env_key, env_value) in compiler.env() { + // MSVC compilers require a handful of environment variables to be set + // to work properly, so they can find their built-in include folders and + // default library path. + cmd.env(env_key, env_value); + } + if target.contains("ios") { // newer iOS deviced have 16kb page sizes: // closed: https://github.com/gnzlbg/jemallocator/issues/68 @@ -301,9 +433,7 @@ fn main() { run_and_log(&mut cmd, &build_dir.join("config.log")); // Make: - let make = make_cmd(&host); - run(Command::new(make) - .current_dir(&build_dir) + run(make_cmd(&host, &build_dir, &compiler) .arg("-j") .arg(num_jobs.clone())); @@ -311,18 +441,17 @@ fn main() { if env::var("JEMALLOC_SYS_RUN_JEMALLOC_TESTS").is_ok() { info!("Building and running jemalloc tests..."); // Make tests: - run(Command::new(make) - .current_dir(&build_dir) + run(make_cmd(&host, &build_dir, &compiler) .arg("-j") .arg(num_jobs.clone()) .arg("tests")); // Run tests: - run(Command::new(make).current_dir(&build_dir).arg("check")); + run(make_cmd(&host, &build_dir, &compiler).arg("check")); } // Make install: - run(Command::new(make) + run(make_cmd(&host, &build_dir, &compiler) .current_dir(&build_dir) .arg("install_lib_static") .arg("install_include") @@ -339,7 +468,7 @@ fn main() { // intrinsics that are libgcc specific (e.g. those intrinsics aren't present in // libcompiler-rt), so link that in to get that support. if target.contains("windows") { - println!("cargo:rustc-link-lib=static=jemalloc"); + println!("cargo:rustc-link-lib=static=jemalloc_s"); } else { println!("cargo:rustc-link-lib=static=jemalloc_pic"); } @@ -395,8 +524,15 @@ fn execute(cmd: &mut Command, on_fail: impl FnOnce()) { fn gnu_target(target: &str) -> String { match target { - "i686-pc-windows-msvc" => "i686-pc-win32".to_string(), - "x86_64-pc-windows-msvc" => "x86_64-pc-win32".to_string(), + // We need to lie and pretend to be mingw32 for MSVC builds. Autoconf + // will then check if the compiler is MSVC, and if so, switch to an + // MSVC-based build. + // + // See https://github.com/jemalloc/jemalloc/blob/5.3.0/configure.ac#L750 + "i686-pc-windows-msvc" => "i686-pc-mingw32".to_string(), + "i686-win7-windows-msvc" => "i686-pc-mingw32".to_string(), + "x86_64-pc-windows-msvc" => "x86_64-pc-mingw32".to_string(), + "x86_64-win7-windows-msvc" => "x86_64-pc-mingw32".to_string(), "i686-pc-windows-gnu" => "i686-w64-mingw32".to_string(), "x86_64-pc-windows-gnu" => "x86_64-w64-mingw32".to_string(), "armv7-linux-androideabi" => "arm-linux-androideabi".to_string(), @@ -406,7 +542,7 @@ fn gnu_target(target: &str) -> String { } } -fn make_cmd(host: &str) -> &'static str { +fn make_cmd_program(host: &str) -> &'static str { const GMAKE_HOSTS: &[&str] = &[ "bitrig", "dragonfly", @@ -424,6 +560,28 @@ fn make_cmd(host: &str) -> &'static str { } } +fn make_cmd(host: &str, build_dir: &Path, compiler: &Tool) -> Command { + let mut make = Command::new(make_cmd_program(host)); + make.current_dir(build_dir); + + for (env_key, env_value) in compiler.env() { + // MSVC compilers require a handful of environment variables to be set + // to work properly, so they can find their built-in include folders and + // default library path. + make.env(env_key, env_value); + } + + static SHELL_OVERRIDE: OnceLock> = OnceLock::new(); + + // Override SHELL if necessary. We cache the value of the SHELL to avoid + // recomputing it every time we run make. + if let Some(arg) = SHELL_OVERRIDE.get_or_init(|| find_shell_arg(host, build_dir)) { + make.arg(arg); + } + + make +} + struct BackgroundThreadSupport { always_enabled: bool, } diff --git a/jemalloc-sys/configure/configure b/jemalloc-sys/configure/configure index 7c4e6e266..0d8d4b7c9 100755 --- a/jemalloc-sys/configure/configure +++ b/jemalloc-sys/configure/configure @@ -8615,7 +8615,21 @@ _ACEOF -{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing log" >&5 + +# On MSVC, log is an intrinsic that doesn't require libm. However, +# AC_SEARCH_LIBS does not successfully detect this, as it will try to compile +# a program using the wrong signature for log. Newer versions of MSVC CL detects +# this and rejects the program with the following messages. +# +# conftest.c(40): warning C4391: 'char log()': incorrect return type for intrinsic function, expected 'double' +# conftest.c(44): error C2168: 'log': too few actual parameters for intrinsic function +# +# Since log is always available on MSVC (it's been around since the dawn of +# time), we simply always assume it's there if MSVC is detected. +if test "x$je_cv_msvc" = "xyes" ; then + LM= +else + { $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing log" >&5 $as_echo_n "checking for library containing log... " >&6; } if ${ac_cv_search_log+:} false; then : $as_echo_n "(cached) " >&6 @@ -8673,10 +8687,11 @@ else as_fn_error $? "Missing math functions" "$LINENO" 5 fi -if test "x$ac_cv_search_log" != "xnone required" ; then - LM="$ac_cv_search_log" -else - LM= + if test "x$ac_cv_search_log" != "xnone required" ; then + LM="$ac_cv_search_log" + else + LM= + fi fi diff --git a/jemalloc-sys/jemalloc b/jemalloc-sys/jemalloc index e13ca993e..2f15f126a 160000 --- a/jemalloc-sys/jemalloc +++ b/jemalloc-sys/jemalloc @@ -1 +1 @@ -Subproject commit e13ca993e8ccb9ba9847cc330696e02839f328f7 +Subproject commit 2f15f126a028d78bd089e331e603e60229b7558a diff --git a/jemalloc-sys/src/env.rs b/jemalloc-sys/src/env.rs index 2053ad306..dd12c2197 100644 --- a/jemalloc-sys/src/env.rs +++ b/jemalloc-sys/src/env.rs @@ -12,13 +12,18 @@ pub static UNSUPPORTED_TARGETS: &[&str] = &[ pub static UNTESTED_TARGETS: &[&str] = &["openbsd", "msvc"]; /// `jemalloc`'s background_thread support is known not to work on these targets: -pub static NO_BG_THREAD_TARGETS: &[&str] = &["musl"]; +pub static NO_BG_THREAD_TARGETS: &[&str] = &["musl", "windows"]; /// targets that don't support unprefixed `malloc` // “it was found that the `realpath` function in libc would allocate with libc malloc // (not jemalloc malloc), and then the standard library would free with jemalloc free, // causing a segfault.” +// // https://github.com/rust-lang/rust/commit/e3b414d8612314e74e2b0ebde1ed5c6997d28e8d // https://github.com/rust-lang/rust/commit/9f3de647326fbe50e0e283b9018ab7c41abccde3 // https://github.com/rust-lang/rust/commit/ed015456a114ae907a36af80c06f81ea93182a24 -pub static NO_UNPREFIXED_MALLOC_TARGETS: &[&str] = &["android", "dragonfly", "darwin"]; +// +// Furthermore, macos (using macho abi) and windows (using pecoff abi) don't +// support unprefixed malloc at all: +// https://github.com/jemalloc/jemalloc/blob/8dc97b11089be6d58a52009ea3da610bf90331d3/configure.ac#L1109 +pub static NO_UNPREFIXED_MALLOC_TARGETS: &[&str] = &["android", "dragonfly", "darwin", "windows"]; diff --git a/jemalloc-sys/src/lib.rs b/jemalloc-sys/src/lib.rs index 79096a0f8..078096dba 100644 --- a/jemalloc-sys/src/lib.rs +++ b/jemalloc-sys/src/lib.rs @@ -387,9 +387,9 @@ extern "C" { /// The behavior is _undefined_ if: /// /// * `size` is not in range `[req_size, alloc_size]`, where `req_size` is - /// the size requested when performing the allocation, and `alloc_size` is - /// the allocation size returned by [`nallocx`], [`sallocx`], or - /// [`xallocx`], + /// the size requested when performing the allocation, and `alloc_size` is + /// the allocation size returned by [`nallocx`], [`sallocx`], or + /// [`xallocx`], /// * `ptr` does not match a pointer earlier returned by the memory /// allocation functions of this crate, or /// * `ptr` is null, or @@ -453,8 +453,8 @@ extern "C" { /// Returns `0` on success, otherwise returns: /// /// * `EINVAL`: if `newp` is not null, and `newlen` is too large or too - /// small. Alternatively, `*oldlenp` is too large or too small; in this case - /// as much data as possible are read despite the error. + /// small. Alternatively, `*oldlenp` is too large or too small; in this case + /// as much data as possible are read despite the error. /// /// * `ENOENT`: `name` or mib specifies an unknown/invalid value. /// @@ -463,7 +463,7 @@ extern "C" { /// * `EAGAIN`: A memory allocation failure occurred. /// /// * `EFAULT`: An interface with side effects failed in some way not - /// directly related to `mallctl` read/write processing. + /// directly related to `mallctl` read/write processing. /// /// [jemalloc_mallctl]: http://jemalloc.net/jemalloc.3.html#mallctl_namespace #[cfg_attr(prefixed, link_name = "_rjem_mallctl")] diff --git a/jemallocator/tests/background_thread_enabled.rs b/jemallocator/tests/background_thread_enabled.rs index 76d286d6d..e37c5b222 100644 --- a/jemallocator/tests/background_thread_enabled.rs +++ b/jemallocator/tests/background_thread_enabled.rs @@ -2,7 +2,7 @@ //! library was compiled with background thread run-time support. #![cfg(feature = "background_threads_runtime_support")] #![cfg(not(feature = "unprefixed_malloc_on_supported_platforms"))] -#![cfg(not(target_env = "musl"))] +#![cfg(not(any(windows, target_env = "musl")))] use tikv_jemallocator::Jemalloc;