Skip to content

Commit

Permalink
Code cleanup based on review feedback.
Browse files Browse the repository at this point in the history
- Remove 'runtime' argument.
- Remove 'initialized' and 'heap_allocated' members.
- Use _Py_IsMainInterpreter()
- Remove spurious whitespace change.
- Return -1 on error and 0 on success, as normal convention.
  • Loading branch information
nascheme committed Jan 26, 2024
1 parent f8a93d4 commit bbaffbe
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 32 deletions.
11 changes: 2 additions & 9 deletions Include/internal/pycore_obmalloc.h
Original file line number Diff line number Diff line change
Expand Up @@ -668,15 +668,6 @@ struct _obmalloc_state {
#if WITH_PYMALLOC_RADIX_TREE
struct _obmalloc_usage usage;
#endif
// true if the obmalloc state has been initialized. This must be done
// before the malloc/free functions of obmalloc are called and must be
// done only once per obmalloc state. The function _PyMem_init_obmalloc()
// does the initialization.
bool initialized;
// true if this structure is heap allocated, by PyMem_RawCalloc(). For
// the main interpreter, this structure is statically allocated (in the
// BSS). Using the BSS gives some performance win.
bool heap_allocated;
};


Expand All @@ -695,6 +686,8 @@ extern Py_ssize_t _Py_GetGlobalAllocatedBlocks(void);
_Py_GetGlobalAllocatedBlocks()
extern Py_ssize_t _PyInterpreterState_GetAllocatedBlocks(PyInterpreterState *);
extern void _PyInterpreterState_FinalizeAllocatedBlocks(PyInterpreterState *);
extern int _PyMem_init_obmalloc(PyInterpreterState *interp);
extern bool _PyMem_obmalloc_state_on_heap(PyInterpreterState *interp);


#ifdef WITH_PYMALLOC
Expand Down
59 changes: 43 additions & 16 deletions Objects/obmalloc.c
Original file line number Diff line number Diff line change
Expand Up @@ -973,6 +973,7 @@ typedef struct _obmalloc_state OMState;
* will be allocated in the BSS which is a small performance win. The radix
* tree arrays are fairly large but are sparsely used. */
static struct _obmalloc_state obmalloc_state_main;
static bool obmalloc_state_initialized;

static inline int
has_own_state(PyInterpreterState *interp)
Expand Down Expand Up @@ -1075,7 +1076,7 @@ _PyInterpreterState_FinalizeAllocatedBlocks(PyInterpreterState *interp)
Py_ssize_t leaked = _PyInterpreterState_GetAllocatedBlocks(interp);
assert(has_own_state(interp) || leaked == 0);
interp->runtime->obmalloc.interpreter_leaks += leaked;
if (interp->obmalloc->heap_allocated && leaked == 0) {
if (_PyMem_obmalloc_state_on_heap(interp) && leaked == 0) {
// free the obmalloc arenas and radix tree nodes. If leaked > 0
// then some of the memory allocated by obmalloc has not been
// freed. It might be safe to free the arenas in that case but
Expand Down Expand Up @@ -2633,6 +2634,7 @@ _PyObject_DebugDumpAddress(const void *p)
_PyMem_DumpTraceback(fileno(stderr), p);
}


static size_t
printone(FILE *out, const char* msg, size_t value)
{
Expand Down Expand Up @@ -2683,32 +2685,57 @@ _PyDebugAllocatorStats(FILE *out,
(void)printone(out, buf2, num_blocks * sizeof_block);
}

int _PyMem_init_obmalloc(PyInterpreterState *interp, _PyRuntimeState *runtime)
// Return true if the obmalloc state structure is heap allocated,
// by PyMem_RawCalloc(). For the main interpreter, this structure
// allocated in the BSS. Allocating that way gives some memory savings
// and a small performance win (at least on a demand paged OS). On
// 64-bit platforms, the obmalloc structure is 256 kB. Most of that
// memory is for the arena_map_top array. Since normally only one entry
// of that array is used, only one page of resident memory is actually
// used, rather than the full 256 kB.
bool _PyMem_obmalloc_state_on_heap(PyInterpreterState *interp)
{
#if WITH_PYMALLOC
return interp->obmalloc && interp->obmalloc != &obmalloc_state_main;
#else
return false;
#endif
}

#ifdef WITH_PYMALLOC
static void
init_obmalloc_pools(PyInterpreterState *interp)
{
// initialize the obmalloc->pools structure. This must be done
// before the obmalloc alloc/free functions can be called.
poolp temp[OBMALLOC_USED_POOLS_SIZE] =
_obmalloc_pools_INIT(interp->obmalloc->pools);
memcpy(&interp->obmalloc->pools.used, temp, sizeof(temp));
}
#endif /* WITH_PYMALLOC */

int _PyMem_init_obmalloc(PyInterpreterState *interp)
{
#ifdef WITH_PYMALLOC
/* Initialize obmalloc, but only for subinterpreters,
since the main interpreter is initialized statically. */
if (interp == &runtime->_main_interpreter
|| (interp->feature_flags & Py_RTFLAGS_USE_MAIN_OBMALLOC)) {
if (_Py_IsMainInterpreter(interp)
|| _PyInterpreterState_HasFeature(interp,
Py_RTFLAGS_USE_MAIN_OBMALLOC)) {
interp->obmalloc = &obmalloc_state_main;
interp->obmalloc->heap_allocated = false;
if (!obmalloc_state_initialized) {
init_obmalloc_pools(interp);
obmalloc_state_initialized = true;
}
} else {
interp->obmalloc = PyMem_RawCalloc(1, sizeof(struct _obmalloc_state));
if (interp->obmalloc == NULL) {
return 0;
return -1;
}
interp->obmalloc->heap_allocated = true;
}
if (!interp->obmalloc->initialized) {
// initialize the obmalloc->pools structure. This must be done
// before the obmalloc alloc/free functions can be called.
poolp temp[OBMALLOC_USED_POOLS_SIZE] =
_obmalloc_pools_INIT(interp->obmalloc->pools);
memcpy(&interp->obmalloc->pools.used, temp, sizeof(temp));
interp->obmalloc->initialized = true;
init_obmalloc_pools(interp);
}
#endif /* WITH_PYMALLOC */
return 1;
return 0; // success
}


Expand Down
9 changes: 3 additions & 6 deletions Python/pylifecycle.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "pycore_typevarobject.h" // _Py_clear_generic_types()
#include "pycore_unicodeobject.h" // _PyUnicode_InitTypes()
#include "pycore_weakref.h" // _PyWeakref_GET_REF()
#include "pycore_obmalloc.h" // _PyMem_init_obmalloc()

#include "opcode.h"

Expand Down Expand Up @@ -606,10 +607,6 @@ init_interp_create_gil(PyThreadState *tstate, int gil)
}


// defined in obmalloc.c
int _PyMem_init_obmalloc(PyInterpreterState *interp, _PyRuntimeState *runtime);


static PyStatus
pycore_create_interpreter(_PyRuntimeState *runtime,
const PyConfig *src_config,
Expand Down Expand Up @@ -646,7 +643,7 @@ pycore_create_interpreter(_PyRuntimeState *runtime,
// initialize the interp->obmalloc state. This must be done after
// the settings are loaded (so that feature_flags are set) but before
// any calls are made to obmalloc functions.
if (!_PyMem_init_obmalloc(interp, runtime)) {
if (_PyMem_init_obmalloc(interp) < 0) {
return _PyStatus_NO_MEMORY();
}

Expand Down Expand Up @@ -2135,7 +2132,7 @@ new_interpreter(PyThreadState **tstate_p, const PyInterpreterConfig *config)
// initialize the interp->obmalloc state. This must be done after
// the settings are loaded (so that feature_flags are set) but before
// any calls are made to obmalloc functions.
if (!_PyMem_init_obmalloc(interp, runtime)) {
if (_PyMem_init_obmalloc(interp) < 0) {
status = _PyStatus_NO_MEMORY();
goto error;
}
Expand Down
3 changes: 2 additions & 1 deletion Python/pystate.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "pycore_pystate.h"
#include "pycore_runtime_init.h" // _PyRuntimeState_INIT
#include "pycore_sysmodule.h" // _PySys_Audit()
#include "pycore_obmalloc.h" // _PyMem_obmalloc_state_on_heap()

/* --------------------------------------------------------------------------
CAUTION
Expand Down Expand Up @@ -547,7 +548,7 @@ free_interpreter(PyInterpreterState *interp)
// The main interpreter is statically allocated so
// should not be freed.
if (interp != &_PyRuntime._main_interpreter) {
if (interp->obmalloc && interp->obmalloc->heap_allocated) {
if (_PyMem_obmalloc_state_on_heap(interp)) {
// interpreter has its own obmalloc state, free it
PyMem_RawFree(interp->obmalloc);
interp->obmalloc = NULL;
Expand Down

0 comments on commit bbaffbe

Please sign in to comment.