mimalloc: additional integration and changes for --disable-gil builds

[colesbury]

Feature or enhancement

Mimalloc was added as an allocator in #90815. The --disable-gil builds need further integration with mimalloc, as well as some modifications to mimalloc to support thread-safe garbage collection in --disable-gil builds and the dictionary accesses that mostly avoid locking.

These changes can be split up across multiple PRs.

• Currently, when mimalloc is enabled, all allocations go to the default heap. This is fine for PyMem_Malloc calls, but we need separate heaps for PyObject_Malloc and PyObject_GC_New. We should associate some mi_heap_ts with each PyThreadState. Every PyThreadState needs four heaps: one for PyMem_Malloc, one for non-GC objects (via PyObject_Malloc), one for GC objects with managed dicts (extra pre-header) and one for GC objects without a managed dict. We need some way to know which heap to use in _PyObject_MiMalloc. There's not a great way to do this, but I suggest adding something like a "current pyobject heap" variable to PyThreadState. It should generally point to the PyObject_Malloc heap, but PyObject_GC_New should temporarily override it to point to the correct GC heap when called
• --disable-gil should imply --with-mimalloc and require mimalloc (i.e., disallow changing the allocator with PYTHONMALLOC).
• We should tag each mi_heap_t and mi_page_t with a number identifying which type of allocation it's associated with. This is important for when pages are abandoned (i.e., when a thread exits with live blocks remaining) and the page is no longer associated with a heap. The GC still needs to identify which of those pages store GC-enabled objects. (see colesbury/nogil-3.12@d447b69)
• When claiming a page from an abandoned segment, mimalloc should associate it with the correct heap from the current thread. In other words, pages that store GC-enabled objects should only be put back in the correct GC heap.

cc @DinoV

Linked PRs

• gh-112532: Require mimalloc in --disable-gil builds #112883
• gh-112532: Use separate mimalloc heaps for GC objects #113263
• gh-112532: Fix peg generator build.py for mimalloc build #113492
• gh-112532: Isolate abandoned segments by interpreter #113717
• gh-112532: Tag mimalloc heaps and pages #113742
• gh-112532: Fix memory block count for free-threaded build #113995
• gh-112532: Improve mimalloc page visiting #114133

[encukou]

The AMD64 Ubuntu NoGIL Refleaks buildbot has been failing for two weeks now, and it looks like it was caused by #113263 (separate heaps). I'll try to reproduce and investigate further.

[corona10]

@colesbury cc @encukou

The AMD64 Ubuntu NoGIL Refleaks buildbot has been failing for two weeks now, and it looks like it was caused by #113263 (separate heaps). I'll try to reproduce and investigate further.

I double-checked the leak on my local macOS with bisecting, and acf3bcc is triggered commit.

./python.exe -m test test_dict -R 3:3

[encukou]

I minimized a few of the failing test cases. I found that all pass random strings to either pathlib or eval:

click for the 4 minimized cases

import pathlib
import os
import tempfile
import unittest

class ModifiedZipAppTest(unittest.TestCase):
    def test_create_archive(self):
        for i in range(6): # need six or more to trigger the leak
            tmpdir_p = tempfile.TemporaryDirectory()
            pathlib.Path(tmpdir_p.name).joinpath('source').mkdir()
            tmpdir_p.cleanup()

import secrets
import string
import unittest

class ModifiedDictTest(unittest.TestCase):
    def test_set_constructor(self):
        n = 6  # need 6 or more to trigger the leak

        # Generate a set of `n` unique strings
        items = {(''.join(secrets.token_hex(5))) for i in range(n)}
        dictliteral = str(items)

        # Eval'ing the set's repr leaks
        eval(dictliteral)

import secrets
import unittest

from collections import namedtuple

class ModifiedTestNamedTuple(unittest.TestCase):

    def test_odd_sizes(self):
        n = 6  # need 6 or more to trigger leak
        names = ['s' + secrets.token_hex(5) for i in range(n)]
        namedtuple('Big', names)

import tempfile
import os
import pathlib

import unittest


class ModifiedTempfileTest(unittest.TestCase):
    def test_choose_directory(self):
        for i in range(6):  # you guessed it, 6 again
            dir = tempfile.mkdtemp()
            tempfile._infer_return_type(pathlib.Path(dir))
            os.rmdir(dir)

What do eval and pathlib have in common? Turns out they both use sys.intern nowadays. Now that's a much more likely refleak culprit :)

Small Python reproducer:

import secrets
import gc
import sys

before = 0
for i in range(30):
    sys.intern(secrets.token_hex(2))

    gc.collect()
    now = sys.gettotalrefcount()
    print(f'{now}: {now-before:+}')
    before = now

Per PEP 703, interned strings are immortalized. That might ... not be a good idea? It'll definitely need a doc update. IMO, the PEP should be more clear that this is a change -- the other immortal objects it lists are already immortal in the GIL build.

edit: but there might be more to this...

[colesbury]

I suspect this has something to do with _PyInterpreterState_GetAllocatedBlocks() not checking the correct heaps in --disable-gil builds. I'll check if fixing _PyInterpreterState_GetAllocatedBlocks() fixes the refleak errors.

[colesbury]

Ok, so fixing get_num_global_allocated_blocks and _PyInterpreterState_GetAllocatedBlocks fixes the refleak error.

However, the the failure was for reference counts, not leaked memory blocks so it's strange that fixing the memory block count fixes the purported reference leak.

I think there's also something buggy in refleak.py related to the refcounts on ints. Either the get_pooled_int() logic isn't sufficient, or we are hiding some actual reference leak elsewhere

[corona10]

@colesbury Leakage from test_socket had been already happened before #113263 was merged, I checked with commit 8f5b998

[corona10]

@encukou cc @python/macos-team
The test_socket leakage on Apple silicon macOS has been happening for a long time ago even for the default build.
We just did not detect this issue since there are no refleak builders for macOS at all.
python/buildmaster-config#444 (comment)

[corona10]

I will create a new issue for this.

[corona10]

#114049 is created.

[colesbury]

I think think this is done now. There are still a few QSBR-related mimalloc changes, but those will be tracked in #115103.