Skip to content

Conversation

@Aniketsy
Copy link
Contributor

@Aniketsy Aniketsy commented Dec 24, 2025

  • Update SimpleQueue.sizeof() to include the memory used by its internal buffer.
  • add a unit test.

Please let me know if my approach or fix needs any improvements . I’m open to feedback and happy to make changes based on suggestions.
Thankyou !

Copy link
Contributor

@colesbury colesbury left a comment

Choose a reason for hiding this comment

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

A few comments below. Please also add a blurb

Comment on lines 241 to 243
static PyObject *
simplequeue_sizeof(PyObject *op, PyObject *Py_UNUSED(ignored))
{
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs a critical section (a lock for the queue). The simplest way is to use "Argument Clinic" like the other functions.

Something like:

/*[clinic input]
@critical_section
_queue.SimpleQueue.__sizeof__ -> Py_ssize_t

Returns size in memory, in bytes.
[clinic start generated code]*/

static Py_ssize_t
simplequeue_sizeof_impl(simplequeueobject *self)
/*[clinic end generated code: output=3303f008eaa6a0a5 input=9b51620c76fc4507]*/

Then run make clinic to regenerate the bindings.

_QUEUE_SIMPLEQUEUE_PUT_METHODDEF
_QUEUE_SIMPLEQUEUE_PUT_NOWAIT_METHODDEF
_QUEUE_SIMPLEQUEUE_QSIZE_METHODDEF
{"__sizeof__", (PyCFunction)simplequeue_sizeof, METH_NOARGS, NULL},
Copy link
Contributor

Choose a reason for hiding this comment

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

You'll also need to change this to use the Argument Clinic methoddef (like above).


def test_simplequeue_sizeof_reflects_buffer_growth(self):
q = self.type2test()
before = q.__sizeof__()
Copy link
Contributor

Choose a reason for hiding this comment

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

A few changes:

  1. Don't use ctypes (no support for that in wasi)
  2. Use support.calcobjsize
  3. Use sys.getsizeof() instead of calling __sizeof__ directly

@Aniketsy
Copy link
Contributor Author

@colesbury thanks for the feedback. i've applied changes as per suggestions, please let me know if this needs further improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants