Skip to content

gh-112529: Track if debug allocator is used as underlying allocator #113747

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jan 16, 2024
Merged

gh-112529: Track if debug allocator is used as underlying allocator #113747

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4734274
gh-112529: Track if debug allocator is used as underlying allocator
colesbury Jan 5, 2024
3a07f3a
Simplify _PyMem_DebugEnabled
colesbury Jan 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Include/internal/pycore_pymem.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ struct _pymem_allocators {
debug_alloc_api_t mem;
debug_alloc_api_t obj;
} debug;
int is_debug_enabled;
PyObjectArenaAllocator obj_arena;
};

Expand Down Expand Up @@ -106,6 +107,8 @@ extern int _PyMem_GetAllocatorName(
PYMEM_ALLOCATOR_NOT_SET does nothing. */
extern int _PyMem_SetupAllocators(PyMemAllocatorName allocator);

/* Is the debug allocator enabled? */
extern int _PyMem_DebugEnabled(void);

#ifdef __cplusplus
}
Expand Down
2 changes: 2 additions & 0 deletions Include/internal/pycore_pymem_init.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,15 @@ extern void _PyMem_ArenaFree(void *, void *, size_t);
PYDBGMEM_ALLOC(runtime), \
PYDBGOBJ_ALLOC(runtime), \
}
# define _pymem_is_debug_enabled_INIT 1
#else
# define _pymem_allocators_standard_INIT(runtime) \
{ \
PYRAW_ALLOC, \
PYMEM_ALLOC, \
PYOBJ_ALLOC, \
}
# define _pymem_is_debug_enabled_INIT 0
#endif

#define _pymem_allocators_debug_INIT \
Expand Down
1 change: 1 addition & 0 deletions Include/internal/pycore_runtime_init.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ extern PyTypeObject _PyExc_MemoryError;
.standard = _pymem_allocators_standard_INIT(runtime), \
.debug = _pymem_allocators_debug_INIT, \
.obj_arena = _pymem_allocators_obj_arena_INIT, \
.is_debug_enabled = _pymem_is_debug_enabled_INIT, \
}, \
.obmalloc = _obmalloc_global_state_INIT, \
.pyhash_state = pyhash_state_INIT, \
Expand Down
25 changes: 19 additions & 6 deletions Objects/obmalloc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -439,12 +439,14 @@ set_up_allocators_unlocked(PyMemAllocatorName allocator)
(void)set_default_allocator_unlocked(PYMEM_DOMAIN_RAW, pydebug, NULL);
(void)set_default_allocator_unlocked(PYMEM_DOMAIN_MEM, pydebug, NULL);
(void)set_default_allocator_unlocked(PYMEM_DOMAIN_OBJ, pydebug, NULL);
_PyRuntime.allocators.is_debug_enabled = pydebug;
break;

case PYMEM_ALLOCATOR_DEBUG:
(void)set_default_allocator_unlocked(PYMEM_DOMAIN_RAW, 1, NULL);
(void)set_default_allocator_unlocked(PYMEM_DOMAIN_MEM, 1, NULL);
(void)set_default_allocator_unlocked(PYMEM_DOMAIN_OBJ, 1, NULL);
_PyRuntime.allocators.is_debug_enabled = 1;
break;

#ifdef WITH_PYMALLOC
Expand All @@ -458,7 +460,9 @@ set_up_allocators_unlocked(PyMemAllocatorName allocator)
set_allocator_unlocked(PYMEM_DOMAIN_MEM, &pymalloc);
set_allocator_unlocked(PYMEM_DOMAIN_OBJ, &pymalloc);

if (allocator == PYMEM_ALLOCATOR_PYMALLOC_DEBUG) {
int is_debug = (allocator == PYMEM_ALLOCATOR_PYMALLOC_DEBUG);
_PyRuntime.allocators.is_debug_enabled = is_debug;
if (is_debug) {
set_up_debug_hooks_unlocked();
}
break;
Expand All @@ -477,7 +481,9 @@ set_up_allocators_unlocked(PyMemAllocatorName allocator)
PyMemAllocatorEx objmalloc = MIMALLOC_OBJALLOC;
set_allocator_unlocked(PYMEM_DOMAIN_OBJ, &objmalloc);

if (allocator == PYMEM_ALLOCATOR_MIMALLOC_DEBUG) {
int is_debug = (allocator == PYMEM_ALLOCATOR_MIMALLOC_DEBUG);
_PyRuntime.allocators.is_debug_enabled = is_debug;
if (is_debug) {
set_up_debug_hooks_unlocked();
}

Expand All @@ -493,7 +499,9 @@ set_up_allocators_unlocked(PyMemAllocatorName allocator)
set_allocator_unlocked(PYMEM_DOMAIN_MEM, &malloc_alloc);
set_allocator_unlocked(PYMEM_DOMAIN_OBJ, &malloc_alloc);

if (allocator == PYMEM_ALLOCATOR_MALLOC_DEBUG) {
int is_debug = (allocator == PYMEM_ALLOCATOR_MALLOC_DEBUG);
_PyRuntime.allocators.is_debug_enabled = is_debug;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these assignments necessary? It seems like this is going to get set in set_up_debug_hooks_unlocked anyway. I suppose there's a question if you were to change the allocator after initializing it, but it seems like that would blow up horribly anyway?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think they're not necessary - I'll remove them and make sure things still work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out they are needed. We may statically initialize is_debug_enabled to 1 in a debug build, but then disable debug hooks via configuration (such as, PYTHONMALLOC=mimalloc). In that case we need to clear is_debug_enabled.

if (is_debug) {
set_up_debug_hooks_unlocked();
}
break;
Expand Down Expand Up @@ -604,13 +612,17 @@ _PyMem_GetCurrentAllocatorName(void)
}


#ifdef WITH_PYMALLOC
static int
int
_PyMem_DebugEnabled(void)
{
return (_PyObject.malloc == _PyMem_DebugMalloc);
#ifdef WITH_PYMALLOC
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like this ifdef isn't necessary and i should always reutnr is_debug_enabled? This flag gets set regardless if Python is built w/ pymalloc or without. It also seems like maybe builds with the GIL disabled will ultimately want to build w/o PYMALLOC given that it's not suitable for them?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that makes sense -- I'll get rid of the #ifdef.

return _PyRuntime.allocators.is_debug_enabled;
#else
return 0;
#endif
}

#ifdef WITH_PYMALLOC
static int
_PyMem_PymallocEnabled(void)
{
Expand Down Expand Up @@ -691,6 +703,7 @@ set_up_debug_hooks_unlocked(void)
set_up_debug_hooks_domain_unlocked(PYMEM_DOMAIN_RAW);
set_up_debug_hooks_domain_unlocked(PYMEM_DOMAIN_MEM);
set_up_debug_hooks_domain_unlocked(PYMEM_DOMAIN_OBJ);
_PyRuntime.allocators.is_debug_enabled = 1;
}

void
Expand Down