Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update mimalloc to v2.1.2 #4740

Merged
merged 8 commits into from
Dec 21, 2023
Merged

Conversation

dscho
Copy link
Member

@dscho dscho commented Dec 19, 2023

The mimalloc project has released a couple versions since we last integrated it into Git for Windows. Let's take the most recent one.

This PR is structured in the following way:

  1. Revert all mimalloc commits, in reverse chronological order (newest first), marking them as fixup!s of the reverted commits.
  2. Import (a subset of) mimalloc v2.1.2, marking it as an amend! of the commit that originally imported mimalloc.
  3. Replay the remaining mimalloc commits, in chronological order, marking them as fixup!s of the original commits.

dscho and others added 8 commits December 19, 2023 10:32
Revert this in preparation for upgrading mimalloc to v2.1.2 and then
redoing this patch.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Revert this in preparation for upgrading mimalloc to v2.1.2 and then
redoing this patch.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Revert this in preparation for upgrading mimalloc to v2.1.2 and then
redoing this patch.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
In preparation for upgrading mimalloc to v2.1.2, let's revert this patch
so that we can start afresh, from a clean slate.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Import the source code of mimalloc v2.1.2

This commit imports mimalloc's source code as per v2.1.2, fetched from
the tag at https://github.com/microsoft/mimalloc.

The .c files are from the src/ subdirectory, and the .h files from the
include/ and include/mimalloc/ subdirectories. We will subsequently
modify the source code to accommodate building within Git's context.

Since we plan on using the `mi_*()` family of functions, we skip the
C++-specific source code, some POSIX compliant functions to interact
with mimalloc, and the code that wants to support auto-magic overriding
of the `malloc()` function (mimalloc-new-delete.h, alloc-posix.c,
mimalloc-override.h, alloc-override.c, alloc-override-osx.c,
alloc-override-win.c and static.c).

To appease the `check-whitespace` job of Git's Continuous Integration,
this commit was washed one time via `git rebase --whitespace=fix`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
We want to compile mimalloc's source code as part of Git, rather than
requiring the code to be built as an external library: mimalloc uses a
CMake-based build, which is not necessarily easy to integrate into the
flavors of Git for Windows (which will be the main benefitting port).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
By defining `USE_MIMALLOC`, Git can now be compiled with that
nicely-fast and small allocator.

Note that we have to disable a couple `DEVELOPER` options to build
mimalloc's source code, as it makes heavy use of declarations after
statements, among other things that disagree with Git's conventions.

We even have to silence some GCC warnings in non-DEVELOPER mode. For
example, the `-Wno-array-bounds` flag is needed because in `-O2` builds,
trying to call `NtCurrentTeb()` (which `_mi_thread_id()` does on
Windows) causes the bogus warning about a system header, likely related
to https://sourceforge.net/p/mingw-w64/mailman/message/37674519/ and to
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578:

C:/git-sdk-64-minimal/mingw64/include/psdk_inc/intrin-impl.h:838:1:
        error: array subscript 0 is outside array bounds of 'long long unsigned int[0]' [-Werror=array-bounds]
  838 | __buildreadseg(__readgsqword, unsigned __int64, "gs", "q")
      | ^~~~~~~~~~~~~~

Also: The `mimalloc` library uses C11-style atomics, therefore we must
require that standard when compiling with GCC if we want to use
`mimalloc` (instead of requiring "only" C99). This is what we do in the
CMake definition already, therefore this commit does not need to touch
`contrib/buildsystems/`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Always use the internal "use_weak" random seed when initializing
the "mimalloc" heap when statically linked on Windows.

The imported "mimalloc" routines support several random sources
to seed the heap data structures, including BCrypt.dll and
RtlGenRandom.  Crashes have been reported when using BCrypt.dll
if it initialized during an `atexit()` handler function.  Granted,
such DLL initialization should not happen in an atexit handler,
but yet the crashes remain.

It should be noted that on Windows when statically linked, the
mimalloc startup code (called by the GCC CRT to initialize static
data prior to calling `main()`) always uses the internal "weak"
random seed.  "mimalloc" does not try to load an alternate
random source until after the OS initialization has completed.

Heap data is stored in `__declspec(thread)` TLS data and in theory
each Git thread will have its own heap data.  However, testing
shows that the "mimalloc" library doesn't actually call
`os_random_buf()` (to load a new random source) when creating these
new per-thread heap structures.

However, if an atexit handler is forced to run on a non-main
thread, the "mimalloc" library *WILL* try to create a new heap
and seed it with `os_random_buf()`.  (The reason for this is still
a mystery to this author.)  The `os_random_buf()` call can cause
the (previously uninitialized BCrypt.dll library) to be dynamically
loaded and a call made into it.  Crashes have been reported in
v2.40.1.vfs.0.0 while in this call.

As a workaround, the fix here forces the use of the internal
"use_weak" random code for the subsequent `os_random_buf()` calls.
Since we have been using that random generator for the majority
of the program, it seems safe to use it for the final few mallocs
in the atexit handler (of which there really shouldn't be that many.

Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho dscho self-assigned this Dec 19, 2023
@dscho dscho marked this pull request as ready for review December 19, 2023 11:53
@dscho dscho added this to the Next release milestone Dec 21, 2023
@dscho dscho merged commit 9cdc478 into git-for-windows:main Dec 21, 2023
40 checks passed
@dscho dscho deleted the update-mimalloc-to-v2.1.2 branch December 21, 2023 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants