[PATCH 01 of 11] bdiff: use Python memory allocator in fixws

[Gregory Szorc]

> On Mar 9, 2017, at 18:24, Augie Fackler <raf at durin42.com> wrote:
> 
> 
>> On Mar 9, 2017, at 18:12, Gregory Szorc <gregory.szorc at gmail.com> wrote:
>> 
>> On Thu, Mar 9, 2017 at 5:46 PM, Augie Fackler <raf at durin42.com> wrote:
>>> On Thu, Mar 09, 2017 at 01:59:09PM -0800, Gregory Szorc wrote:
>>> # HG changeset patch
>>> # User Gregory Szorc <gregory.szorc at gmail.com>
>>> # Date 1489089265 28800
>>> #      Thu Mar 09 11:54:25 2017 -0800
>>> # Node ID 7a8ce919a13a9ff6e73f391fe14d228e18387f15
>>> # Parent  1871a1ee64ed49172b1568b89cdbab126284b309
>>> bdiff: use Python memory allocator in fixws
>>> 
>>> Python has its own memory allocation APIs. For allocations
>>> <= 512 bytes, it allocates memory from arenas. This means that
>>> average small allocations don't call the system allocator, which
>>> makes them faster. Also, arena allocations cut down on memory
>>> fragmentation, which can matter for performance in long-running
>>> processes.
>>> 
>>> Another advantage of using the Python memory allocator is that
>>> allocations are tracked by Python. This is a bigger deal in
>>> Python 3, as modern versions of Python have some decent built-in
>>> tools for examining memory usage, leaks, etc.
>> 
>> This is all true...for PyObject* functions. PyMem_Malloc is a trivial
>> wrapper macro around malloc:
>> 
>> https://hg.python.org/cpython/file/v2.7.10/Include/pymem.h#l74
>> 
>> (YMMV if PYMALLOC_DEBUG is defined, but I don't think that's the default)
>> 
>> OTOH, at least this explains why you couldn't see a performance change?
>> 
>> Indeed. I've apparently been living too much in Python 3 lately, as Python 3's PyMem_Malloc() *does* use the arena-based "pymalloc" allocator for small allocations.
> 
> Only in some build configurations - do you know what the defaults are? (In particular, -DWITH_PYMALLOC at compile time, see https://github.com/python/cpython/blob/master/Objects/obmalloc.c#L153 - it's a bit of a maze of structs and preprocessor hoops.)

It looks PyMem_Malloc didn't switch to the arena allocator for small allocations until 3.6: https://bugs.python.org/issue26249.

I'll have to spend some time to dig through the source for the history of all of this and what allocators are used on which systems. Apparently Windows might be different because the Windows allocator is better for small allocations...

> 
> I'm hesitant to move towards cpython integrated allocation functions because it might make future fuzzing or pypy work more difficult.
> 
>> 
>> PyObject_Malloc() does exist in Python 2 and does use arena-based allocations. But very few things in CPython use it and it is undocumented in the C API docs. So we should probably stay away.
>> 
>> Since the patches are no-op for Python 2 and represent a positive change in Python 3, do you want to just queue them with an in-flight commit message change to add "in Python 3?" I can resend just part 1 with that change if you want.
>> 
>> 
>>> 
>>> This patch converts a trivial malloc() + free() in the bdiff code
>>> to use the Python allocator APIs. Since the object being
>>> operated on is a line, chances are it will use an arena. So,
>>> this could have a net positive impact on performance (although
>>> I didn't measure it).
>>> 
>>> diff --git a/mercurial/bdiff_module.c b/mercurial/bdiff_module.c
>>> --- a/mercurial/bdiff_module.c
>>> +++ b/mercurial/bdiff_module.c
>>> @@ -158,7 +158,7 @@ static PyObject *fixws(PyObject *self, P
>>>      r = PyBytes_AsString(s);
>>>      rlen = PyBytes_Size(s);
>>> 
>>> -     w = (char *)malloc(rlen ? rlen : 1);
>>> +     w = (char *)PyMem_Malloc(rlen ? rlen : 1);
>>>      if (!w)
>>>              goto nomem;
>>> 
>>> @@ -178,7 +178,7 @@ static PyObject *fixws(PyObject *self, P
>>>      result = PyBytes_FromStringAndSize(w, wlen);
>>> 
>>> nomem:
>>> -     free(w);
>>> +     PyMem_Free(w);
>>>      return result ? result : PyErr_NoMemory();
>>> }
>>> 
>>> _______________________________________________
>>> Mercurial-devel mailing list
>>> Mercurial-devel at mercurial-scm.org
>>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170311/bdcc7cf4/attachment.html>