[issue26249] Change PyMem_Malloc to use pymalloc allocator

[STINNER Victor]

STINNER Victor added the comment:

There are no more know bugs related to this change, I close the issue. Thanks 
for the test_format report Serhiy, I missed it.

--
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[STINNER Victor]

STINNER Victor added the comment:

> 68b2a43d8653 introduced memory leak.

I was very surprised to see a regression in test_format since I didn't change 
any change related to bytes, bytearray or str formatting in this issue.

In fact, it's much better than that! With PyMem_Malloc() using pymalloc, we 
benefit for free of the cheap "_Py_AllocatedBlocks" memory leak detector. I 
introduced the memory leak in the issue #25349 when I optimimzed bytes%args and 
bytearray%args using the new _PyBytesWriter API.

This memory leak gave me an idea, I opened the issue #26850: 
"PyMem_RawMalloc(): update also sys.getallocatedblocks() in debug mode".

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[Roundup Robot]

Roundup Robot added the comment:

New changeset 090502a0c69c by Victor Stinner in branch 'default':
Issue #25349, #26249: Fix memleak in formatfloat()
https://hg.python.org/cpython/rev/090502a0c69c

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

68b2a43d8653 introduced memory leak.

$ ./python -m test.regrtest -uall -R : test_format
Run tests sequentially
0:00:00 [1/1] test_format
beginning 9 repetitions
123456789
.
test_format leaked [6, 7, 7, 7] memory blocks, sum=27
1 test failed:
test_format
Total duration: 0:00:01

--
status: closed -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[STINNER Victor]

STINNER Victor added the comment:

I documented the change, buildbots are happy, I close the issue.

--
resolution:  -> fixed
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[Roundup Robot]

Roundup Robot added the comment:

New changeset 7acad5d8f80e by Victor Stinner in branch 'default':
Issue #26249: Mention PyMem_Malloc() change in What's New in Python 3.6 in the
https://hg.python.org/cpython/rev/7acad5d8f80e

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[Roundup Robot]

Roundup Robot added the comment:

New changeset 104ed24ebbd0 by Victor Stinner in branch 'default':
Issue #26249: Try test_capi on Windows
https://hg.python.org/cpython/rev/104ed24ebbd0

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[Roundup Robot]

Roundup Robot added the comment:

New changeset 68b2a43d8653 by Victor Stinner in branch 'default':
PyMem_Malloc() now uses the fast pymalloc allocator
https://hg.python.org/cpython/rev/68b2a43d8653

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[STINNER Victor]

STINNER Victor added the comment:

I created the issue #26563 "PyMem_Malloc(): check that the GIL is hold in debug 
hooks".

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[STINNER Victor]

STINNER Victor added the comment:

> Using the issue #26516 (PYTHONMALLOC=debug), we can check PyGILState_Check() 
> at runtime, but there is currently an issue related to sub-interpreters. The 
> assertion fails in support.run_in_subinterp(), function used by 
> test_threading and test_capi for example.

I created #26558 to implement GIL checks in PyMem_Malloc() and 
PyObject_Malloc().

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[STINNER Victor]

STINNER Victor added the comment:

pymalloc.patch: Updated patch.

--
Added file: http://bugs.python.org/file42158/pymalloc.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[STINNER Victor]

STINNER Victor added the comment:

> Does the API doc say anything about the GIL, for example?

I modified Python to add assert(PyGILState_Check()); in PyMem_Malloc() and 
other functions.

Sadly, I found a bug in Numpy: Numpy releases the GIL for performance but call 
PyMem_Malloc() with the GIL released. I proposed a fix:
https://github.com/numpy/numpy/pull/7404

I guess that the fix is obvious and will be quickly merged, but it means that 
other libraries may have the issue.

Using the issue #26516 (PYTHONMALLOC=debug), we can check PyGILState_Check() at 
runtime, but there is currently an issue related to sub-interpreters. The 
assertion fails in support.run_in_subinterp(), function used by test_threading 
and test_capi for example.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[Yury Selivanov]

Changes by Yury Selivanov :


--
nosy:  -Yury.Selivanov, yselivanov

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[STINNER Victor]

STINNER Victor added the comment:

2016-03-09 18:28 GMT+01:00 Antoine Pitrou :
> Does the API doc say anything about the GIL, for example? Or Valgrind?

For the GIL, yes, Python 3 doc is explicit:
https://docs.python.org/dev/c-api/memory.html#memory-interface

Red and bold warning: "The GIL must be held when using these functions."

Hum, sadly it looks like the warning miss in Python 2 doc.

The GIL was the motivation to introduce the PyMem_RawMalloc() function
in Python 3.4.

For Valgrind: using the issue #26516, you will be able to use
PYTHONMALLOC=malloc to use easily Valgrind even on a Python compiled
in release mode (which is a new feature, before you had to manually
recompile Python in debug mode with --with-valgrind)).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[STINNER Victor]

STINNER Victor added the comment:

cryptography: good

* Git commit 0681de7241dcbaec7b3dc85d3cf3944e4bec8309 (Mar 9 2016)

"4 failed, 77064 passed, 3096 skipped in 405.09 seconds" 

1 error is related to the version number (probably an issue on how I run the 
tests), 3 errors are FileNotFoundError related to cryptography_vectors. At 
least, there is no Python fatal error related to memory allocators ;-)

--

Hum, just in case, I checked my venv:

(ENV) haypo@smithers$ python -c 'import _testcapi; _testcapi.pymem_api_misuse()'
...
Fatal Python error: bad ID: Allocated using API 'o', verified using API 'r'

(ENV) haypo@smithers$ python -c 'import _testcapi; 
_testcapi.pymem_buffer_overflow()'
...
Fatal Python error: bad trailing pad byte

It works ;-)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[Antoine Pitrou]

Antoine Pitrou added the comment:

Le 09/03/2016 18:27, STINNER Victor a écrit :
> 
> I disagree on the fact that my change breaks any API. The API doc is
clear.

Does the API doc say anything about the GIL, for example? Or Valgrind?

> I suggest you to continue the discussion on python-dev for a wider
audience. I will test a few more projects before replying on the
python-dev thread.

I have no interest in going back and forth between the Python tracker
and python-dev (especially since I hardly read python-dev these days).
If you address my questions positively here I will be happy with the patch!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[STINNER Victor]

STINNER Victor added the comment:

> I'm talking about the performance improvement in third-party libraries, not 
> the performance improvement in CPython itself which can be addressed by 
> replacing the internal API calls.

Oh ok. I don't know how to measure the performance of third-party libraries. I 
expect no speedup or a little speedup, but no slow-down.


> Do you think it's reasonable to risk breaking external libraries just
for a hypothetic "performance improvement"?

The question is if my change really breaks anything in practice. I'm testing 
some popular C extensions to prepare an answer. Early results is that developer 
use correctly the Python allocator API :-)

I disagree on the fact that my change breaks any API. The API doc is clear. For 
example, you must use PyMem_Free() on memory allocated by PyMem_Malloc(). If 
you use free(), it fails badly with Python compiled in debug mode.

My issue #26516 "Add PYTHONMALLOC env var and add support for malloc debug 
hooks in release mode" may help developers to validate their own application.

I suggest you to continue the discussion on python-dev for a wider audience. I 
will test a few more projects before replying on the python-dev thread.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[Antoine Pitrou]

Antoine Pitrou added the comment:

> Yes. It was discussed in the python-dev thread.

I'm talking about the performance improvement in third-party libraries, not the 
performance improvement in CPython itself which can be addressed by replacing 
the internal API calls.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[STINNER Victor]

STINNER Victor added the comment:

> Define "slow".  malloc() on Linux should be reasonably fast.

See first messages of this issue for benchmark results. Some specific 
benchmarks are faster, none is slower.


> Do you think it's reasonable to risk breaking external libraries just
for a hypothetic "performance improvement"?

Yes. It was discussed in the python-dev thread.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[Antoine Pitrou]

Antoine Pitrou added the comment:

Le 09/03/2016 18:01, STINNER Victor a écrit :
> I don't understand why we should keep a slow allocator if Python has a faster 
> allocator?

Define "slow".  malloc() on Linux should be reasonably fast.

Do you think it's reasonable to risk breaking external libraries just
for a hypothetic "performance improvement"?

Again, why don't you try simply changing internal calls?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[STINNER Victor]

STINNER Victor added the comment:

Pillow: good

Note: I had to install JPEG headers (sudo dnf install -y libjpeg-turbo-devel).

Tested version: git commit 44c5cfc3874deaac9cfa87780822ee714c0d (Mar 8 
2016).

---
Pillow$ python setup.py install
Pillow$ python selftest.py
Pillow$ python  test-installed.py
(...)
Ran 671 tests in 8.458s

FAILED (SKIP=124, errors=2)
---

The two errors are "OSError: decoder libtiff not available".

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[STINNER Victor]

STINNER Victor added the comment:

Antoine Pitrou added the comment:
> Victor, why do you insist on this instead of changing internal API calls in 
> CPython?

https://mail.python.org/pipermail/python-dev/2016-February/143097.html

"There are 536 calls to the functions PyMem_Malloc(), PyMem_Realloc()
and PyMem_Free()."

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[STINNER Victor]

STINNER Victor added the comment:

lxml: good!

* I patched Python 3.6 with pymem.patch of this issue + pymem-3.patch of issue 
#26516
* Tested lxml version: git commit 93ec66f6533995a7742278f9ba14b925149ac140 (Mar 
8 2016)

lxml$ make test
(...)
Ran 1735 tests in 27.663s

OK

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[Antoine Pitrou]

Antoine Pitrou added the comment:

> "There are 536 calls to the functions PyMem_Malloc(), PyMem_Realloc()
and PyMem_Free()."

I'm sure you can use powerful tools such as "sed" ;-)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[STINNER Victor]

STINNER Victor added the comment:

> I'm sure you can use powerful tools such as "sed" ;-)

I guess that PyMem functions are used in third party C extensions modules. I 
expect (minor) speedup in these modules too.

I don't understand why we should keep a slow allocator if Python has a faster 
allocator?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[STINNER Victor]

STINNER Victor added the comment:

numpy: good!

* I patched Pyhon 3.6 with pymem.patch of this issue + pymem-3.patch of issue 
#26516
* I had issues to run tests with Python 3.6 compiled in debug mode: 
http://bugs.python.org/issue26519 & https://github.com/numpy/numpy/issues/7399
* I ran the test suite: all tests pass, no bug related to memory allocators
* Tested numpy version: commit b92cc76afad2e74cbbf6f5b9f5b68050f7c8642a (Mar 7 
2016)

Commands ran in numpy tests in a virtual environment:

numpy$ python setup.py install
numpy$ cd..
$ python -c 'import numpy; numpy.test()'
(...)
Ran 6206 tests in 280.986s

OK (KNOWNFAIL=7, SKIP=6)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[Antoine Pitrou]

Antoine Pitrou added the comment:

Victor, why do you insist on this instead of changing internal API calls in 
CPython?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[STINNER Victor]

STINNER Victor added the comment:

In february 2016, I started a thread on the python-dev mailing list:
[Python-Dev] Modify PyMem_Malloc to use pymalloc for performance
https://mail.python.org/pipermail/python-dev/2016-February/143084.html

M.-A. Lemburg wrote:

"""

> Do you see any drawback of using pymalloc for PyMem_Malloc()?

Yes: You cannot free memory allocated using pymalloc with the
standard C lib free().

It would be better to go through the list of PyMem_*() calls
in Python and replace them with PyObject_*() calls, where
possible.

> Does anyone recall the rationale to have two families to memory allocators?

The PyMem_*() APIs were needed to have a cross-platform malloc()
implementation which returns standard C lib free()able memory,
but also behaves well when passing 0 as size.
"""


M.-A. Lemburg fears that the PyMem_Malloc() API is misused:

"""

Sometimes, yes, but we also do allocations for e.g.
parsing values in Python argument tuples (e.g. using
"es" or "et"):

https://docs.python.org/3.6/c-api/arg.html

We do document to use PyMem_Free() on those; not sure whether
everyone does this though.
"""


M.-A. Lemburg suggested to the patch of this issue on:

"""
Yes, but those are part of the stdlib. You'd need to check
a few C extensions which are not tested as part of the stdlib,
e.g. numpy, scipy, lxml, pillow, etc. (esp. ones which implement custom
types in C since these will often need the memory management
APIs).

It may also be a good idea to check wrapper generators such
as cython, swig, cffi, etc.
"""

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[STINNER Victor]

Changes by STINNER Victor :


Removed file: http://bugs.python.org/file42103/pymem-3.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[STINNER Victor]

STINNER Victor added the comment:

Patch 3:

- Ooops, I updated pymem_api_misuse(), but I forgot to update the related unit 
test. It's now fixed.

--
Added file: http://bugs.python.org/file42103/pymem-3.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[STINNER Victor]

STINNER Victor added the comment:

I created the issue #26516 "Add PYTHONMALLOC env var and add support for malloc 
debug hooks in release mode" to help developers to detect bugs in their code, 
especially misuse of the PyMem_Malloc() API.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26249] Change PyMem_Malloc to use pymalloc allocator

[STINNER Victor]

Changes by STINNER Victor :


--
title: Change PyMem_Malloc to use PyObject_Malloc allocator? -> Change 
PyMem_Malloc to use pymalloc allocator

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com