classification
Title: Stop using the garbage collector to manage the lifetime of the getargs.c freelist
Type: enhancement Stage: resolved
Components: Versions:
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: exarkun Nosy List: eli.bendersky, exarkun, python-dev, skrah
Priority: normal Keywords: patch

Created on 2012-03-15 18:22 by exarkun, last changed 2012-03-16 18:26 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
getargs.patch exarkun, 2012-03-15 19:10 review
leak.py skrah, 2012-03-16 16:56
Messages (7)
msg155926 - (view) Author: Jean-Paul Calderone (exarkun) * (Python committer) Date: 2012-03-15 18:22
Allocating a Python list and a bunch of Capsules for each PyArg_ParseTuple call is expensive and unnecessarily complicated.

The freelist never escapes getargs.c (if it ever did, it would be a bug).  The same job can be accomplished with a normal C array.
msg156020 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2012-03-16 12:52
New changeset 9fc456ac20cf by Jean-Paul Calderone in branch 'default':
Issue #14325: Stop using python lists, capsules, and the garbage collector to deal with PyArg_Parse* cleanup.
http://hg.python.org/cpython/rev/9fc456ac20cf
msg156022 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2012-03-16 13:05
This is cool! Any benchmark / speedup data?
msg156024 - (view) Author: Jean-Paul Calderone (exarkun) * (Python committer) Date: 2012-03-16 13:07
If it's a bit faster, that'd be a nice win, but I didn't benchmark.  I'm primarily interested in correctness in the PyPy case (PyPy re-uses this code), and I think CPython benefits from the slightly simplified code as well.

If you do any benchmarks, I'd love to hear the results, though. :)
msg156057 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2012-03-16 16:55
I'm getting a leak since this revision:

./configure --without-pymalloc CFLAGS="-O0 -g" && make
valgrind --db-attach=yes --suppressions=./Misc/valgrind-python.supp --leak-check=full ./python leak.py

==32303== 16 bytes in 1 blocks are definitely lost in loss record 3 of 2,489
==32303==    at 0x4C27878: malloc (vg_replace_malloc.c:236)
==32303==    by 0x41DDB0: PyMem_Malloc (object.c:1841)
==32303==    by 0x4A72BF: vgetargskeywords (getargs.c:1432)
==32303==    by 0x4A6E1C: PyArg_ParseTupleAndKeywords (getargs.c:1301)
==32303==    by 0x55D1C8: float_new (floatobject.c:1557)
==32303==    by 0x42B643: type_call (typeobject.c:708)
==32303==    by 0x532A9F: PyObject_Call (abstract.c:2150)
==32303==    by 0x491483: do_call (ceval.c:4260)
==32303==    by 0x490A33: call_function (ceval.c:4063)
==32303==    by 0x48BD49: PyEval_EvalFrameEx (ceval.c:2662)
==32303==    by 0x48EA11: PyEval_EvalCodeEx (ceval.c:3414)
==32303==    by 0x482FC6: PyEval_EvalCode (ceval.c:771)
msg156058 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2012-03-16 16:56
Here's leak.py.
msg156067 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2012-03-16 18:26
New changeset d08f0f3ab23e by Benjamin Peterson in branch 'default':
plug memory leak (closes #14325)
http://hg.python.org/cpython/rev/d08f0f3ab23e
History
Date User Action Args
2012-03-16 18:26:10python-devsetstatus: open -> closed

messages: + msg156067
stage: resolved
2012-03-16 16:56:41skrahsetfiles: + leak.py

messages: + msg156058
2012-03-16 16:55:21skrahsetstatus: closed -> open
nosy: + skrah
messages: + msg156057

2012-03-16 13:07:57exarkunsetmessages: + msg156024
2012-03-16 13:05:29eli.benderskysetnosy: + eli.bendersky
messages: + msg156022
2012-03-16 12:53:54exarkunsetstatus: open -> closed
resolution: fixed
2012-03-16 12:52:00python-devsetnosy: + python-dev
messages: + msg156020
2012-03-15 19:10:03exarkunsetfiles: + getargs.patch
keywords: + patch
2012-03-15 18:22:39exarkuncreate