bpo-36051: drop GIL during large b''.join operations #17757

bmerry · 2019-12-30T15:46:40Z

Improve multi-threaded performance by dropping the GIL in the fast path
of bytes.join. To avoid increasing overhead for small joins, it is only
done if the output size exceeds a threshold.

https://bugs.python.org/issue36051


        bpo-36051: drop GIL during large b''.join operations

Improve multi-threaded performance by dropping the GIL in the fast path of bytes.join. To avoid increasing overhead for small joins, it is only done if the output size exceeds a threshold.

bmerry · 2019-12-30T16:55:55Z

For now I've only made the change for the b''.join fast-path, but if there is agreement I could also add it to the general case (non-empty separator).

I think the code is safe against another thread mutating the sequence while the GIL is dropped (because the setup code takes it own refs to all the items), but it would be good if someone could double-check it.


        bpo-36051: add extra tests for join()

The existing stress test is increased in size to exceed GIL_THRESHOLD, and a new stress test is added with an empty separator (since that is a separate code path).


        bpo-36051: adjustments to bytes.join GIL-dropping

The GIL is now also dropped in the code path with non-empty separator, but it is *not* dropped if any of the items is not a `bytes` object (see discussion in bpo-36051 for reasoning).


        bpo-36051: increase threshold to 1048576

Benchmarking shows that for smaller thresholds the multi-threading performance tends to be worse, or at least not much better.

Objects/stringlib/join.h


        [bpo-36051] Style fix: add braces

methane · 2020-01-28T10:31:00Z

Misc/NEWS.d/next/Core and Builtins/2019-12-30-15-56-07.bpo-36051.imaVlq.rst

@@ -0,0 +1 @@
+Drop the GIL during large ``bytes.join`` operations.


Add your name in ACKS (see https://devguide.python.org/pullrequest/#crediting),
and add "Patch by ...." here.

I've already added myself to ACKS, but have now added "Patch by ...".

I can not find "Bruce Merry" in ACKS.
https://github.com/python/cpython/blob/master/Misc/ACKS

That's the master version of ACKS. I'm adding myself in this pull request: https://github.com/bmerry/cpython/blob/bpo-36051/Misc/ACKS#L1109


        [bpo-36051] Add name credit to NEWS.d.

bedevere-bot · 2020-01-29T08:02:17Z

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot PPC64LE Fedora 3.x has failed when building commit d07d9f4.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/11/builds/259) and take a look at the build logs.
Check if the failure is related to this commit (d07d9f4) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/11/builds/259

Failed tests:

test_largefile

Failed subtests:

test_it - test.test_largefile.TestCopyfile
test_it - test.test_largefile.TestSocketSendfile
setUpClass - test.test_asyncio.test_base_events.BaseLoopSockSendfileTests

Summary of the results of the build (if available):

== Tests result: FAILURE then FAILURE ==

406 tests OK.

10 slowest tests:

test_mailbox: 8 min 18 sec
test_shelve: 6 min 14 sec
test_multiprocessing_spawn: 2 min 51 sec
test_concurrent_futures: 2 min 45 sec
test_posix: 2 min 40 sec
test_capi: 2 min 35 sec
test_tokenize: 2 min 26 sec
test_unparse: 2 min 24 sec
test_lib2to3: 2 min 7 sec
test_multiprocessing_forkserver: 1 min 55 sec

1 test failed:
test_largefile

13 tests skipped:
test_devpoll test_ioctl test_kqueue test_msilib test_ossaudiodev
test_startfile test_tix test_tk test_ttk_guionly test_winconsoleio
test_winreg test_winsound test_zipfile64

2 re-run tests:
test_asyncio test_largefile

Total duration: 42 min 12 sec

Click to see traceback logs

Traceback (most recent call last):
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64le/build/Lib/test/test_largefile.py", line 195, in run
    f.write(chunk)
OSError: [Errno 28] No space left on device


Traceback (most recent call last):
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64le/build/Lib/test/test_largefile.py", line 161, in test_it
    shutil.copyfile(TESTFN, TESTFN2)
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64le/build/Lib/shutil.py", line 270, in copyfile
    _fastcopy_sendfile(fsrc, fdst)
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64le/build/Lib/shutil.py", line 163, in _fastcopy_sendfile
    raise err from None
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64le/build/Lib/shutil.py", line 149, in _fastcopy_sendfile
    sent = os.sendfile(outfd, infd, offset, blocksize)
OSError: [Errno 28] No space left on device: '@test_38097_tmp' -> '@test_38097_tmp2'


Traceback (most recent call last):
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64le/build/Lib/test/test_largefile.py", line 161, in test_it
    shutil.copyfile(TESTFN, TESTFN2)
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64le/build/Lib/shutil.py", line 270, in copyfile
    _fastcopy_sendfile(fsrc, fdst)
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64le/build/Lib/shutil.py", line 163, in _fastcopy_sendfile
    raise err from None
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64le/build/Lib/shutil.py", line 149, in _fastcopy_sendfile
    sent = os.sendfile(outfd, infd, offset, blocksize)
OSError: [Errno 28] No space left on device: '@test_10521_tmp' -> '@test_10521_tmp2'


Traceback (most recent call last):
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64le/build/Lib/test/test_largefile.py", line 209, in test_it
    client.sendfile(f)
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64le/build/Lib/socket.py", line 483, in sendfile
    return self._sendfile_use_sendfile(file, offset, count)
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64le/build/Lib/socket.py", line 400, in _sendfile_use_sendfile
    raise err from None
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64le/build/Lib/socket.py", line 386, in _sendfile_use_sendfile
    sent = os_sendfile(sockno, fileno, offset, blocksize)
BrokenPipeError: [Errno 32] Broken pipe


Traceback (most recent call last):
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64le/build/Lib/test/test_asyncio/test_base_events.py", line 1986, in setUpClass
    with open(support.TESTFN, 'wb') as fp:
OSError: [Errno 28] No space left on device: '@test_43782_tmp'


Traceback (most recent call last):
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64le/build/Lib/threading.py", line 944, in _bootstrap_inner
    self.run()
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64le/build/Lib/threading.py", line 882, in run
    self._target(*self._args, **self._kwargs)
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64le/build/Lib/test/test_largefile.py", line 195, in run
    f.write(chunk)
OSError: [Errno 28] No space left on device

bpo-36051: drop GIL during large b''.join operations

Loading status checks…

03d07fd

Improve multi-threaded performance by dropping the GIL in the fast path of bytes.join. To avoid increasing overhead for small joins, it is only done if the output size exceeds a threshold.

the-knights-who-say-ni added the CLA signed label Dec 30, 2019

bedevere-bot added the awaiting review label Dec 30, 2019

bmerry added 3 commits Dec 31, 2019

bpo-36051: add extra tests for join()

0feabcf

The existing stress test is increased in size to exceed GIL_THRESHOLD, and a new stress test is added with an empty separator (since that is a separate code path).

bpo-36051: adjustments to bytes.join GIL-dropping

Loading status checks…

4e783c1

The GIL is now also dropped in the code path with non-empty separator, but it is *not* dropped if any of the items is not a `bytes` object (see discussion in bpo-36051 for reasoning).

bpo-36051: increase threshold to 1048576

Loading status checks…

4e636d0

Benchmarking shows that for smaller thresholds the multi-threading performance tends to be worse, or at least not much better.

bmerry requested a review from methane Jan 27, 2020

methane reviewed Jan 28, 2020

View changes

Objects/stringlib/join.h Show resolved Hide resolved

[bpo-36051] Style fix: add braces

Loading status checks…

f538066

methane reviewed Jan 28, 2020

View changes

[bpo-36051] Add name credit to NEWS.d.

Loading status checks…

8fe5d12

bedevere-bot removed the awaiting review label Jan 29, 2020

bmerry deleted the bmerry:bpo-36051 branch Jan 29, 2020

python / cpython

bpo-36051: drop GIL during large b''.join operations #17757

bpo-36051: drop GIL during large b''.join operations #17757

bmerry commented Dec 30, 2019 •

edited by bedevere-bot

This comment has been minimized.

bmerry commented Dec 30, 2019

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bedevere-bot commented Jan 29, 2020

python / cpython

Sponsor python/cpython

Join GitHub today

bpo-36051: drop GIL during large b''.join operations #17757

bpo-36051: drop GIL during large b''.join operations #17757

Conversation

bmerry commented Dec 30, 2019 • edited by bedevere-bot

This comment has been minimized.

bmerry commented Dec 30, 2019

This comment has been minimized.

methane Jan 28, 2020

This comment has been minimized.

bmerry Jan 29, 2020

This comment has been minimized.

methane Jan 29, 2020

This comment has been minimized.

bmerry Jan 29, 2020

This comment has been minimized.

bedevere-bot commented Jan 29, 2020

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

bmerry commented Dec 30, 2019 •

edited by bedevere-bot