Repository navigation
Creating a greenlet does an mmap and munmap each time #534
Description
Hi, I noticed that creating greenlets got a bit slower when I tried moving from Python 3.14 to 3.15, and had a look at why. I don't know greenlet's internals well, so apologies if I've got any of this wrong.
This loop:
import time
import greenlet
def f():
pass
N = 200_000
t = time.perf_counter()
for _ in range(N):
greenlet.greenlet(f).switch()
print(f"{(time.perf_counter() - t) / N * 1e9:.0f} ns per greenlet")takes about 7.4µs per greenlet on 3.14.8 and 8.1µs on 3.15.0 for us (greenlet 3.5.6, Linux x86_64). Under strace each greenlet does an mmap and a munmap of 16KB, and on 3.15 an madvise as well:
$ strace -c -e trace=mmap,munmap,madvise python3.15 mwe.py
% time seconds usecs/call calls errors syscall
------ ----------- ----------- --------- --------- ----------------
42.71 2.114750 10 200009 munmap
29.31 1.450907 7 200042 mmap
27.98 1.385244 6 200004 madvise
------ ----------- ----------- --------- --------- ----------------
100.00 4.950901 8 600055 total
The madvise is new in 3.15 (CPython now adds MADV_HUGEPAGE to its arena allocations) and accounts for the slowdown. The mmap and munmap happen on 3.14 too though.
As far as I can tell the 16KB is the new greenlet's data stack chunk, which PythonState::did_finish frees when the greenlet finishes. CPython added a one chunk cache per thread for a similar problem in gh-142183 (tstate->datastack_cached_chunk, in 3.15 and backported to 3.13.13 and 3.14.4), and a new greenlet's first frame would pick up a chunk from there if there was one.
I tried giving the finished greenlet's chunk to that cache instead of freeing it:
diff --git a/src/greenlet/TPythonState.cpp b/src/greenlet/TPythonState.cpp
index bdedebe..5137349 100644
--- a/src/greenlet/TPythonState.cpp
+++ b/src/greenlet/TPythonState.cpp
@@ -597,6 +597,19 @@ void PythonState::did_finish(PyThreadState* tstate) noexcept
while (chunk) {
_PyStackChunk *prev = chunk->previous;
chunk->previous = nullptr;
+#if GREENLET_HAS_DATASTACK_CACHE
+ // Hand one chunk to CPython's per-thread cache (gh-142183)
+ // so the next greenlet's first frame push reuses it instead
+ // of mmapping a fresh chunk. The field was appended to the
+ // struct in a patch release, so check the runtime too.
+ if (tstate
+ && !tstate->datastack_cached_chunk
+ && greenlet_runtime_has_datastack_cache()) {
+ tstate->datastack_cached_chunk = chunk;
+ chunk = prev;
+ continue;
+ }
+#endif
alloc.free(alloc.ctx, chunk, chunk->size);
chunk = prev;
}
diff --git a/src/greenlet/greenlet_cpython_compat.hpp b/src/greenlet/greenlet_cpython_compat.hpp
index e76f342..0abfc47 100644
--- a/src/greenlet/greenlet_cpython_compat.hpp
+++ b/src/greenlet/greenlet_cpython_compat.hpp
@@ -41,6 +41,27 @@ https://bugs.python.org/issue46090). Summary of breaking internal changes:
# define GREENLET_PY311 0
#endif
+/*
+ PyThreadState.datastack_cached_chunk (gh-142183) was added in 3.15 and
+ backported to 3.13.13 and 3.14.4, appended to the end of the struct.
+ An extension built against newer headers can still be loaded by an
+ older patch release, so callers must also check
+ greenlet_runtime_has_datastack_cache().
+*/
+#define GREENLET_DATASTACK_CACHE_VERSION(v) \
+ (((v) >= 0x030D0D00 && (v) < 0x030E0000) \
+ || ((v) >= 0x030E0400 && (v) < 0x030F0000) \
+ || (v) >= 0x030F00B1)
+#if GREENLET_DATASTACK_CACHE_VERSION(PY_VERSION_HEX)
+# define GREENLET_HAS_DATASTACK_CACHE 1
+static inline bool greenlet_runtime_has_datastack_cache()
+{
+ return GREENLET_DATASTACK_CACHE_VERSION(Py_Version);
+}
+#else
+# define GREENLET_HAS_DATASTACK_CACHE 0
+#endif
+
#if PY_VERSION_HEX >= 0x30C0000
# define GREENLET_PY312 1With that the loop takes about 0.7µs on 3.14.8 and 0.8µs on 3.15.0, and the per-greenlet mmap/munmap/madvise calls go away. greenlet's tests pass on both, as do the gevent tests I tried. The runtime version check is there because the field was added in a patch release, so I think a wheel built against newer headers could otherwise be loaded by 3.14.0-3.14.3 and write past the end of the struct.
Is this a reasonable approach, or is there a reason the chunk can't be reused that I've missed? I'd be happy to open a PR if it's useful.
Activity