Release Python GIL during WEBP encode

[zt-initech]

Changes proposed in this pull request: release GIL in WEBP encoding.

I don't know if the patch is the right way to do it, but it works for me and it makes webp encoding in threads scale, so even if the patch is bad, please do the feature.

[zt-initech]

Is the codecov/project failure real? Can someone explain how to fix it?

Is there anything I can do to improve the chances of this getting merged?

[hugovk]

Thanks for the PR!

Codecov has been a bit flaky of late, you could merge/rebase from master to include #4444, but looks like the macOS coverage was uploaded fine from this PR.

The Codecov links in the checks lead to HTTP 500 error pages, here's a working link:

https://codecov.io/gh/python-pillow/Pillow/commit/0d5e800d7d4a01076371fca3dbff2e4851cc848b

It still says "Processing...", but I see your changes are covered.

And hopefully someone will review this before the next release in April.

[hugovk]

A small suggestion to match the existing style.

Or how about using these macros?

Py_BEGIN_ALLOW_THREADS
... Do some blocking I/O operation ...
Py_END_ALLOW_THREADS

Which expands to:

PyThreadState *_save;

_save = PyEval_SaveThread();
... Do some blocking I/O operation ...
PyEval_RestoreThread(_save);

https://docs.python.org/3/c-api/init.html#releasing-the-gil-from-extension-code

[hugovk]

Please could you share a small code snippet to demonstrate the performance difference?

[hugovk]

Or actually, ImagingSectionEnter/ImagingSectionLeave already exist in _imaging.c and Imaging.h, so should be no need to redefine them in _webp.c.

[zt-initech]

Yes, I copied the exact use of the helpers (and the helpers themselves) from other code. Unfortunately, without copying those two small functions, the resulting _webp.cpython-37m-x86_64-linux-gnu.so has undefined symbols and won't load, so Pillow doesn't have WEBP format support. I don't know enough about Pillow to know how to fix it correctly (e.g. link _webp against _imaging).

The tester:

import io
import time
from threading import Thread

from PIL import Image

full_path = 'Tests/images/fujifilm.mpo'
im = Image.open(full_path)
im.load()


def task(im, count, times):
    t0 = time.time()
    for i in range(count):
        bio_output = io.BytesIO()
        im.save(bio_output, format='WEBP')
    t1 = time.time()
    times.append(t1 - t0)


for thread_count in (1, 2, 4, 8):
    times = []
    threads = [Thread(target=task, args=(im, 1, times)) for i in range(thread_count)]
    for th in threads:
        th.start()
    for th in threads:
        th.join()
    mean = sum(times) / len(times)
    print("thread count: %d, avg time: %.2f, times: %s" % (thread_count, mean, times))

"""
output before fix:
thread count: 1, avg time: 1.39, times: [1.3878045082092285]
thread count: 2, avg time: 2.63, times: [2.6363396644592285, 2.6253771781921387]
thread count: 4, avg time: 4.42, times: [2.733854293823242, 4.041334390640259, 5.4517176151275635, 5.4615771770477295]
thread count: 8, avg time: 7.48, times: [1.430527925491333, 5.566206932067871, 5.586191415786743, 5.631235361099243, 9.46205759048462, 10.74857234954834, 10.731115579605103, 10.674172639846802]

output after fix:
thread count: 1, avg time: 1.50, times: [1.499457597732544]
thread count: 2, avg time: 1.38, times: [1.3706238269805908, 1.3869695663452148]
thread count: 4, avg time: 1.50, times: [1.4900705814361572, 1.5012726783752441, 1.4493658542633057, 1.5475354194641113]
thread count: 8, avg time: 2.30, times: [2.192249059677124, 2.197439670562744, 2.2884106636047363, 2.3436989784240723, 2.3442444801330566, 2.3294525146484375, 2.2780611515045166, 2.416266441345215]
"""

[hugovk]

Ah yes, the others are all inside the _imaging extension, whereas this is the _webp extension.

[hugovk]

And I get somewhat similar results on a dual-core Mac:

before:
thread count: 1, avg time: 1.67, times: [1.667375087738037]
thread count: 2, avg time: 2.97, times: [2.975780963897705, 2.960355043411255]
thread count: 4, avg time: 4.57, times: [3.158473014831543, 3.1742892265319824, 5.998677968978882, 5.964151620864868]
thread count: 8, avg time: 5.94, times: [1.5735549926757812, 3.005002975463867, 5.281926870346069, 6.744750261306763, 8.298348188400269, 8.351630210876465, 9.766034126281738, 4.513238906860352]

after:
thread count: 1, avg time: 1.71, times: [1.707808017730713]
thread count: 2, avg time: 1.92, times: [1.9138820171356201, 1.9199738502502441]
thread count: 4, avg time: 2.75, times: [2.696030378341675, 2.7911901473999023, 2.771228075027466, 2.75399112701416]
thread count: 8, avg time: 4.92, times: [4.834712028503418, 4.939017057418823, 4.97278094291687, 4.873277902603149, 5.142167091369629, 4.985551118850708, 4.854909181594849, 4.79243016242981]