[issue36139] release GIL on mmap dealloc

[STINNER Victor]

STINNER Victor  added the comment:

Yeah! Roundup is fixed, thanks Ezio!

--

___
Python tracker 

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

[issue36139] release GIL on mmap dealloc

[STINNER Victor]

STINNER Victor  added the comment:

4th attempt (sorry for the spam, it's just to check the Roundup fix!):
https://github.com/python/cpython/pull/12073/files#r263026496

--

___
Python tracker 

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

[issue36139] release GIL on mmap dealloc

[Ezio Melotti]

Ezio Melotti  added the comment:

> Oh wow, that's really strange. I'm sure that I wrote "https://...; URL but my 
> URL became "r263026496">https://...; !?

The links are now fixed (Roundup was getting confused by the rNN, since it 
looks like a SVN revision).

--
nosy: +ezio.melotti
type:  -> performance

___
Python tracker 

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

[issue36139] release GIL on mmap dealloc

[STINNER Victor]

STINNER Victor  added the comment:


New changeset dc078947a5033a048d804e244e847b5844734439 by Victor Stinner 
(Davide Rizzo) in branch 'master':
bpo-36139: Fix mmap_object_dealloc(): hold the GIL to call PyMem_Free() 
(GH-12199)
https://github.com/python/cpython/commit/dc078947a5033a048d804e244e847b5844734439


--

___
Python tracker 

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

[issue36139] release GIL on mmap dealloc

[STINNER Victor]

STINNER Victor  added the comment:

The second commit does fix the regression (I tested manually on Windows with 
Python compiled in debug mode).

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

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

[issue36139] release GIL on mmap dealloc

[Davide Rizzo]

Davide Rizzo  added the comment:

BTW should this be backported to 3.7?

--

___
Python tracker 

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

[issue36139] release GIL on mmap dealloc

[Davide Rizzo]

Davide Rizzo  added the comment:

Thanks Victor. I think this fixes it 
https://github.com/python/cpython/pull/12199

--

___
Python tracker 

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

[issue36139] release GIL on mmap dealloc

[Davide Rizzo]

Change by Davide Rizzo :


--
pull_requests: +12194

___
Python tracker 

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

[issue36139] release GIL on mmap dealloc

[STINNER Victor]

STINNER Victor  added the comment:

"""
r263026496">https://github.com/python/cpython/pull/12073/files#r263026496
"""

Oh wow, that's really strange. I'm sure that I wrote "https://...; URL but my 
URL became "r263026496">https://...; !?

3rd attempt to post the link:
https://github.com/python/cpython/pull/12073/files#r263026496

Anyway, my link points to my comment:

"""


PyMem_Free(m_obj->tagname) is called below without holding the GIL: that's 
illegal. Other move the call to free when the GIL is hold again, or use 
PyMem_RawFree().

"Warning: The GIL must be held when using these functions. "
https://docs.python.org/dev/c-api/memory.html#memory-interface

Note: You can use PYTHONMALLOC=debug or -X dev to reproduce the issue on a 
Python compiled in release mode.
"""

On the #ifdef MS_WINDOWS path of mmap_object_dealloc().

--

___
Python tracker 

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

[issue36139] release GIL on mmap dealloc

[STINNER Victor]

STINNER Victor  added the comment:

The bug:
https://github.com/python/cpython/pull/12073/files#r263026496
(correct link, sorry)

--

If nobody comes up quickly with a fix, I will revert the change to repair 
Windows buildbots: PR 12198, as part of the buildbot policy ("revert on 
failure"). I don't have the bandwidth to dig into this failure.
https://pythondev.readthedocs.io/ci.html#revert-on-fail

--

___
Python tracker 

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

[issue36139] release GIL on mmap dealloc

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +12193
stage: resolved -> patch review

___
Python tracker 

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

[issue36139] release GIL on mmap dealloc

[STINNER Victor]

STINNER Victor  added the comment:

The bug:
https://github.com/python/cpython/pull/12073/files#r263026496

--

___
Python tracker 

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

[issue36139] release GIL on mmap dealloc

[STINNER Victor]

STINNER Victor  added the comment:

This change broke the Windows 7 buildbot:
https://buildbot.python.org/all/#/builders/40/builds/1745

Example:

0:01:34 [ 66/420/1] test_mmap crashed (Exit code 2147483651) -- running: 
test_tools (1 min 22 sec)
Fatal Python error: Python memory allocator called without holding the GIL

Current thread 0x0de4 (most recent call first):
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\test_mmap.py", 
line 645 in test_crasher_on_windows
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\unittest\case.py", 
line 642 in run
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\unittest\case.py", 
line 702 in __call__
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\unittest\suite.py", 
line 122 in run
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\unittest\suite.py", 
line 84 in __call__
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\unittest\suite.py", 
line 122 in run
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\unittest\suite.py", 
line 84 in __call__
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\unittest\suite.py", 
line 122 in run
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\unittest\suite.py", 
line 84 in __call__
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\unittest\runner.py", 
line 176 in run
  File 
"C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\support\__init__.py", 
line 1968 in _run_suite
  File 
"C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\support\__init__.py", 
line 2064 in run_unittest
  File 
"C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\libregrtest\runtest.py", 
line 178 in test_runner
  File 
"C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\libregrtest\runtest.py", 
line 182 in runtest_inner
  File 
"C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\libregrtest\runtest.py", 
line 127 in runtest
  File 
"C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\libregrtest\runtest_mp.py",
 line 68 in run_tests_worker
  File 
"C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\libregrtest\main.py", 
line 600 in _main
  File 
"C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\libregrtest\main.py", 
line 586 in main
  File 
"C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\libregrtest\main.py", 
line 640 in main
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\regrtest.py", 
line 46 in _main
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\regrtest.py", 
line 50 in 
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\runpy.py", line 85 in 
_run_code
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\runpy.py", line 192 in 
_run_module_as_main
Windows fatal exception: code 0x8003

Current thread 0x0de4 (most recent call first):
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\test_mmap.py", 
line 645 in test_crasher_on_windows
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\unittest\case.py", 
line 642 in run
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\unittest\case.py", 
line 702 in __call__
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\unittest\suite.py", 
line 122 in run
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\unittest\suite.py", 
line 84 in __call__
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\unittest\suite.py", 
line 122 in run
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\unittest\suite.py", 
line 84 in __call__
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\unittest\suite.py", 
line 122 in run
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\unittest\suite.py", 
line 84 in __call__
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\unittest\runner.py", 
line 176 in run
  File 
"C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\support\__init__.py", 
line 1968 in _run_suite
  File 
"C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\support\__init__.py", 
line 2064 in run_unittest
  File 
"C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\libregrtest\runtest.py", 
line 178 in test_runner
  File 
"C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\libregrtest\runtest.py", 
line 182 in runtest_inner
  File 
"C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\libregrtest\runtest.py", 
line 127 in runtest
  File 
"C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\libregrtest\runtest_mp.py",
 line 68 in run_tests_worker
  File 
"C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\libregrtest\main.py", 
line 600 in _main
  File 
"C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\libregrtest\main.py", 
line 586 in main
  File 
"C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\libregrtest\main.py", 
line 640 in main
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\regrtest.py", 
line 46 in _main
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\regrtest.py", 
line 50 in 
  File 

[issue36139] release GIL on mmap dealloc

[Benjamin Peterson]

Benjamin Peterson  added the comment:


New changeset bb9593af0ac835b93c2834d44b72fa34e30efed0 by Benjamin Peterson 
(Davide Rizzo) in branch 'master':
closes bpo-36139: release GIL around munmap(). (GH-12073)
https://github.com/python/cpython/commit/bb9593af0ac835b93c2834d44b72fa34e30efed0


--
nosy: +benjamin.peterson
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

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

[issue36139] release GIL on mmap dealloc

[Christian Heimes]

Christian Heimes  added the comment:

The change sounds like a good idea and should be backported, too.

IIRC mmap() performance also depends on MMU and TLB speed. In the past I have 
seen paravirtualized systems with poor MMU performance that caused fork() to be 
slow and Redis to hang.

--
nosy: +christian.heimes, davin
versions: +Python 3.7

___
Python tracker 

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

[issue36139] release GIL on mmap dealloc

[Davide Rizzo]

Davide Rizzo  added the comment:

munmap() of private maps is usually pretty fast but not negligible (2 ms for 
1GB). Shared maps are much slower. For some reason, shared maps explicitly 
backed by POSIX shared memory stand in between but are still pretty slow.

If someone cares about file-backed mmaps I can test those too. I thought this 
is already significant to justify releasing the GIL.


This is on Linux 4.4:


shared anon mmap 1048576 bytes
mmap time 6,393 ns
write time  449,062 ns
munmap time 100,205 ns
--
private anon mmap 1048576 bytes
mmap time 2,168 ns
write time  308,966 ns
munmap time  36,930 ns
--
posix shm + mmap 1048576 bytes
mmap time13,299 ns
write time  369,305 ns
close time1,545 ns
munmap time  26,759 ns
--

shared anon mmap 134217728 bytes
mmap time 4,641 ns
write time   64,508,536 ns
munmap time  13,592,556 ns
--
private anon mmap 134217728 bytes
mmap time 6,116 ns
write time   25,402,084 ns
munmap time 388,976 ns
--
posix shm + mmap 134217728 bytes
mmap time29,034 ns
write time   66,826,645 ns
close time3,707 ns
munmap time   3,475,977 ns
--

shared anon mmap 1073741824 bytes
mmap time11,127 ns
write time  508,227,373 ns
munmap time  94,885,306 ns
--
private anon mmap 1073741824 bytes
mmap time 7,133 ns
write time  199,933,903 ns
munmap time   2,361,036 ns
--
posix shm + mmap 1073741824 bytes
mmap time24,868 ns
write time  527,566,819 ns
close time4,015 ns
munmap time  21,179,674 ns
--

--
Added file: https://bugs.python.org/file48177/unmap.c

___
Python tracker 

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

[issue36139] release GIL on mmap dealloc

[Davide Rizzo]

Davide Rizzo  added the comment:

Yes, this is mmap module. I found this to be slow for posix-shm-backed mmaps. 
Several milliseconds, like 20ms for a 128 MB object. Maybe the same can happen 
with private|anon mmaps? I will follow up with more numbers.

--

___
Python tracker 

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

[issue36139] release GIL on mmap dealloc

[Inada Naoki]

Inada Naoki  added the comment:

Sorry, I messed up.  Your PR is changing mmap module, not obmalloc.

--

___
Python tracker 

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

[issue36139] release GIL on mmap dealloc

[Inada Naoki]

Inada Naoki  added the comment:

Your patch is modifying obmalloc.
It uses MAP_PRIVATE|MAP_ANONYMOUS mmap.
Does it really take log time?  How?

#1572968 is different.  It is mmap module.  It can use file-backed mmap.  It is 
I/O.

--
nosy: +inada.naoki

___
Python tracker 

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

[issue36139] release GIL on mmap dealloc

[Roundup Robot]

Change by Roundup Robot :


--
keywords: +patch
pull_requests: +12092
stage:  -> patch review

___
Python tracker 

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

[issue36139] release GIL on mmap dealloc

[Davide Rizzo]

New submission from Davide Rizzo :

munmap() can take a long time. I think mmap_object_dealloc can trivially 
release the GIL around this operation. Something similar was already mentioned 
in https://bugs.python.org/issue1572968 but a general patch was never provided. 
The dealloc case alone is significant enough to deserve fixing.

--
components: Library (Lib)
messages: 336775
nosy: davide.rizzo
priority: normal
severity: normal
status: open
title: release GIL on mmap dealloc
versions: Python 3.8

___
Python tracker 

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