Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
GH-139951: Fix major GC performance regression#140262
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
2e8d0206c78c01c5836bebde53f41c2030a698e5f5File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| Fixes a regression in GC performance for a growing heap composed mostly of | ||
| small tuples. | ||
| * Counts number of actually tracked objects, instead of trackable objects. | ||
| This ensures that untracking tuples has the desired effect of reducing GC overhead. | ||
| * Does not track most untrackable tuples during creation. | ||
| This prevents large numbers of small tuples causing excessive GCs. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -156,13 +156,26 @@ _PyTuple_MaybeUntrack(PyObject *op) | ||
| _PyObject_GC_UNTRACK(op); | ||
| } | ||
| /* Fast, but conservative check if an object maybe tracked | ||
| May return true for an object that is not tracked, | ||
markshannon marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading. Please reload this page. | ||
| Will always return true for an object that is tracked. | ||
| This is a temporary workaround until _PyObject_GC_IS_TRACKED | ||
| becomes fast and safe to call on non-GC objects. | ||
| */ | ||
| static bool | ||
| maybe_tracked(PyObject *ob) | ||
| { | ||
| return _PyType_IS_GC(Py_TYPE(ob)); | ||
| } | ||
| PyObject * | ||
| PyTuple_Pack(Py_ssize_t n, ...) | ||
| { | ||
| Py_ssize_t i; | ||
| PyObject *o; | ||
| PyObject **items; | ||
| va_list vargs; | ||
| bool track = false; | ||
| if (n == 0){ | ||
| return tuple_get_empty(); | ||
| @@ -177,10 +190,15 @@ PyTuple_Pack(Py_ssize_t n, ...) | ||
| items = result->ob_item; | ||
| for (i = 0; i < n; i++){ | ||
| o = va_arg(vargs, PyObject *); | ||
| if (!track && maybe_tracked(o)){ | ||
| track = true; | ||
| } | ||
| items[i] = Py_NewRef(o); | ||
| } | ||
| va_end(vargs); | ||
| _PyObject_GC_TRACK(result); | ||
| if (track){ | ||
| _PyObject_GC_TRACK(result); | ||
| } | ||
| return (PyObject *)result; | ||
| } | ||
| @@ -377,11 +395,17 @@ PyTuple_FromArray(PyObject *const *src, Py_ssize_t n) | ||
| return NULL; | ||
| } | ||
| PyObject **dst = tuple->ob_item; | ||
| bool track = false; | ||
| for (Py_ssize_t i = 0; i < n; i++){ | ||
| PyObject *item = src[i]; | ||
| if (!track && maybe_tracked(item)){ | ||
| track = true; | ||
| } | ||
| dst[i] = Py_NewRef(item); | ||
| } | ||
| _PyObject_GC_TRACK(tuple); | ||
| if (track){ | ||
| _PyObject_GC_TRACK(tuple); | ||
| } | ||
| return (PyObject *)tuple; | ||
| } | ||
| @@ -396,10 +420,17 @@ _PyTuple_FromStackRefStealOnSuccess(const _PyStackRef *src, Py_ssize_t n) | ||
| return NULL; | ||
| } | ||
| PyObject **dst = tuple->ob_item; | ||
| bool track = false; | ||
| for (Py_ssize_t i = 0; i < n; i++){ | ||
| dst[i] = PyStackRef_AsPyObjectSteal(src[i]); | ||
| PyObject *item = PyStackRef_AsPyObjectSteal(src[i]); | ||
| if (!track && maybe_tracked(item)){ | ||
| track = true; | ||
| } | ||
| dst[i] = item; | ||
| } | ||
| if (track){ | ||
| _PyObject_GC_TRACK(tuple); | ||
| } | ||
| _PyObject_GC_TRACK(tuple); | ||
| return (PyObject *)tuple; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1639,7 +1639,7 @@ assess_work_to_do(GCState *gcstate) | ||
| scale_factor = 2; | ||
| } | ||
| intptr_t new_objects = gcstate->young.count; | ||
| intptr_t max_heap_fraction = new_objects*3/2; | ||
| intptr_t max_heap_fraction = new_objects*2; | ||
sergey-miryanov marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading. Please reload this page. | ||
| intptr_t heap_fraction = gcstate->heap_size / SCAN_RATE_DIVISOR / scale_factor; | ||
| if (heap_fraction > max_heap_fraction){ | ||
| heap_fraction = max_heap_fraction; | ||
| @@ -1654,6 +1654,9 @@ gc_collect_increment(PyThreadState *tstate, struct gc_collection_stats *stats) | ||
| GC_STAT_ADD(1, collections, 1); | ||
| GCState *gcstate = &tstate->interp->gc; | ||
| gcstate->work_to_do += assess_work_to_do(gcstate); | ||
| if (gcstate->work_to_do < 0){ | ||
| return; | ||
| } | ||
| untrack_tuples(&gcstate->young.head); | ||
markshannon marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading. Please reload this page. | ||
| if (gcstate->phase == GC_PHASE_MARK){ | ||
| Py_ssize_t objects_marked = mark_at_start(tstate); | ||
| @@ -1696,7 +1699,6 @@ gc_collect_increment(PyThreadState *tstate, struct gc_collection_stats *stats) | ||
| gc_collect_region(tstate, &increment, &survivors, stats); | ||
| gc_list_merge(&survivors, visited); | ||
| assert(gc_list_is_empty(&increment)); | ||
| gcstate->work_to_do += gcstate->heap_size / SCAN_RATE_DIVISOR / scale_factor; | ||
| gcstate->work_to_do -= increment_size; | ||
| add_stats(gcstate, 1, stats); | ||
| @@ -2286,28 +2288,29 @@ _Py_ScheduleGC(PyThreadState *tstate) | ||
| } | ||
| void | ||
| _PyObject_GC_Link(PyObject *op) | ||
| _Py_TriggerGC(struct _gc_runtime_state *gcstate) | ||
| { | ||
| PyGC_Head *gc = AS_GC(op); | ||
| // gc must be correctly aligned | ||
| _PyObject_ASSERT(op, ((uintptr_t)gc & (sizeof(uintptr_t)-1)) == 0); | ||
| PyThreadState *tstate = _PyThreadState_GET(); | ||
| GCState *gcstate = &tstate->interp->gc; | ||
| gc->_gc_next = 0; | ||
| gc->_gc_prev = 0; | ||
| gcstate->young.count++; /* number of allocated GC objects */ | ||
| gcstate->heap_size++; | ||
| if (gcstate->young.count > gcstate->young.threshold && | ||
| gcstate->enabled && | ||
| gcstate->young.threshold && | ||
| if (gcstate->enabled && | ||
markshannon marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading. Please reload this page.
markshannon marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading. Please reload this page. | ||
| gcstate->young.threshold != 0 && | ||
| !_Py_atomic_load_int_relaxed(&gcstate->collecting) && | ||
| !_PyErr_Occurred(tstate)) | ||
| { | ||
| _Py_ScheduleGC(tstate); | ||
| } | ||
| } | ||
| void | ||
| _PyObject_GC_Link(PyObject *op) | ||
| { | ||
| PyGC_Head *gc = AS_GC(op); | ||
| // gc must be correctly aligned | ||
| _PyObject_ASSERT(op, ((uintptr_t)gc & (sizeof(uintptr_t)-1)) == 0); | ||
| gc->_gc_next = 0; | ||
| gc->_gc_prev = 0; | ||
| } | ||
| void | ||
| _Py_RunGC(PyThreadState *tstate) | ||
| { | ||
| @@ -2414,6 +2417,11 @@ PyObject_GC_Del(void *op) | ||
| PyGC_Head *g = AS_GC(op); | ||
| if (_PyObject_GC_IS_TRACKED(op)){ | ||
| gc_list_remove(g); | ||
| GCState *gcstate = get_gc_state(); | ||
| if (gcstate->young.count > 0){ | ||
| gcstate->young.count--; | ||
| } | ||
| gcstate->heap_size--; | ||
| #ifdef Py_DEBUG | ||
| PyObject *exc = PyErr_GetRaisedException(); | ||
| if (PyErr_WarnExplicitFormat(PyExc_ResourceWarning, "gc", 0, | ||
| @@ -2427,11 +2435,6 @@ PyObject_GC_Del(void *op) | ||
| PyErr_SetRaisedException(exc); | ||
| #endif | ||
| } | ||
| GCState *gcstate = get_gc_state(); | ||
| if (gcstate->young.count > 0){ | ||
| gcstate->young.count--; | ||
| } | ||
| gcstate->heap_size--; | ||
| PyObject_Free(((char *)op)-presize); | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.