Skip to content
Closed
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
fix free bug and remove ifdef in a few places
  • Loading branch information
iritkatriel committed Feb 3, 2023
commit 6b312e0da1acdedd0f104392578a16c249d09345
17 changes: 4 additions & 13 deletions Include/internal/pycore_interp.h
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,7 @@ struct _is {
struct _Py_context_state context;
struct _Py_exc_state exc_state;

#if WITH_FREELISTS
_PyFreeList freelists[INTERP_NUM_FREELISTS];
#endif
struct ast_state ast;
struct types_state types;
struct callable_cache callable_cache;
Expand Down Expand Up @@ -237,30 +235,23 @@ PyAPI_FUNC(int) _PyInterpreterState_IDInitref(PyInterpreterState *);
PyAPI_FUNC(int) _PyInterpreterState_IDIncref(PyInterpreterState *);
PyAPI_FUNC(void) _PyInterpreterState_IDDecref(PyInterpreterState *);

#define SIZE_TO_FREELIST_INDEX(size) ((size-4)/2)
#define FREELIST_INDEX_TO_SIZE(idx) (2*(idx) + 4)
#define FREELIST_QUANTUM (2*sizeof(void*))
#define SIZE_TO_FREELIST_INDEX(size) ((size-4)/FREELIST_QUANTUM)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why this formula?

You want to allocate all sizes from (n-1)*FREELIST_QUANTUM + 1 to n*FREELIST_QUANTUM (inclusive) from the same freelist.
I would expect the formula to be (size + FREELIST_QUANTUM -1)/FREELIST_QUANTUM, or (size-1)/FREELIST_QUANTUM.

And because C division rounds towards 0, not -infinity, you want to use >>LOG_BASE_2_OF_FREELIST_QUANTUM, not /FREELIST_QUANTUM.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I haven't changed this yet. I'm trying to get it to work with just ints.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

There already are some macros in pycore_obmalloc.h that seem to do something like this:

/* 
 * Alignment of addresses returned to the user. 8-bytes alignment works
 * on most current architectures (with 32-bit or 64-bit address buses).
 * The alignment value is also used for grouping small requests in size
 * classes spaced ALIGNMENT bytes apart.
 *
 * You shouldn't change this unless you know what you are doing.
 */

#if SIZEOF_VOID_P > 4
#define ALIGNMENT              16               /* must be 2^N */
#define ALIGNMENT_SHIFT         4
#else
#define ALIGNMENT               8               /* must be 2^N */
#define ALIGNMENT_SHIFT         3
#endif
   
/* Return the number of bytes in size class I, as a uint. */
#define INDEX2SIZE(I) (((pymem_uint)(I) + 1) << ALIGNMENT_SHIFT)

#define FREELIST_INDEX_TO_SIZE(idx) (FREELIST_QUANTUM*(idx) + 4)
Comment thread
iritkatriel marked this conversation as resolved.
Outdated

static inline PyObject*
_PyInterpreterState_FreelistAlloc(PyInterpreterState *interp, Py_ssize_t size) {
#if WITH_FREELISTS
int index = SIZE_TO_FREELIST_INDEX(size);
assert(index >= 0 && index < INTERP_NUM_FREELISTS);
return _PyFreeList_Alloc(&interp->freelists[index]);
#else
return PyObject_Malloc(size);
#endif
}

static inline void
_PyInterpreterState_FreelistFree(PyInterpreterState * interp, PyObject *op, Py_ssize_t size) {
#if WITH_FREELISTS
/* todo: assert the size is correct? */
Comment thread
iritkatriel marked this conversation as resolved.
int index = SIZE_TO_FREELIST_INDEX(size);
assert(index >= 0 && index < INTERP_NUM_FREELISTS);
_PyFreeList_Alloc(&interp->freelists[index]);
#else
Py_TYPE(op)->tp_free((PyObject *)op);
#endif
_PyFreeList_Free(&interp->freelists[index], op);
}


Expand Down