In https://twitter.com/moreati/status/1137815804530049028 Alex Willmer wrote:

In CPython 3.8dev the pure http://pickle.py module now depends on the C _pickle module, but only for one class

cpython/Lib/pickle.py

Line 39 in c879ff2

from _pickle import PickleBuffer

Antoine, would it make sense turn _pickle.PickleBuffer into an optional dependency? The class is only used for pickle 5 as described in PEP-574, https://www.python.org/dev/peps/pep-0574/

I noticed this because I was experimenting with pickle.py from the 3.8 branch to support protocol 5 in https://github.com/moreati/pikl (and later to https://pypi.org/project/zodbpickle/).

However I want to make it clear, if CPython maintainers wish to keep the code as it is I will deal with it. Maximizing our convenience, and minimizing your maintenance workload is paramount.

Arggh, typo. I mean maximizing *your* convenience is paramount.

Note that PickleBuffer is really a built-in type (it's defined in Objects/picklebufobject.c). However it's exposed from _pickle.c because it doesn't make sense to expose it in the builtins module.

In short I'm not against making PickleBuffer optional but I'm not sure it makes a lot of sense. Would you make memoryview optional because it's written in C? Is anyone annoyed by the fact that something comes from _pickle.c.

(note that the pure Python Pickler class *does* work with PickleBuffer; so you don't lose any extensability or customizability by using PickleBuffer)

In any case, I won't do the work myself, so feel free to draft a PR so that we see the magnitude of the changes required.

Attempting a PR

I don't think I can do this. My WIP code is in https://github.com/moreati/cpython/pull/new/bpo-37210, and associated make test output is attached.

Principal blockers

• _pickle.PickleBuffer.raw() can return a contiguous buffer from either a c_contiguous, or f_contiguous buffer. memoryview() (and hence pickle._PickleBuffer.raw()) requires a c_contiguous buffer.
• I'm unsure how to allow _PickleBuffer -> bytearray conversion

The most common cause of test failures is TypeError: cannot pickle 'memoryview' object. I guess Python is seeing a lack of reduce(), and failing back to walking the object attributes. Implementing reduce() and or reduce_ex() might fix this, but seems moot given the other blockers.

Right, it is probably not possible to write a pure Python PickleBuffer. There's a reason it's written in C...

When you said "make PickleBuffer an optional dependency", I thought you intended to have a usable pure Python Pickler, but without support for the PickleBuffer class.

it is probably not possible to write a pure Python PickleBuffer

Fair enough

a usable pure Python Pickler, but without support for the PickleBuffer class.

That makes sense. However, for third party packages (e.g. zodbpickle, pikl) wanting a pure Python pickle module to use as a basis, I think it would make more sense to use the CPython 3.7 branch as a basis. Adding a pickle._Pickler variant to the 3.8 stdlib that can't do protocol 5, just for third-parties imposes a maintenance burden on the core devs for zero benefit to anyone.

I propose closing this ticket as WONTFIX.

Ok, closing then.

FYI pyperformance is affected by this issue:

vstinner@apu$ env/bin/python ~/prog/python/pyperformance/pyperformance/benchmarks/bm_pickle.py unpickle --pure-python -v -p3  
Traceback (most recent call last):
  File "/home/vstinner/prog/python/pyperformance/pyperformance/benchmarks/bm_pickle.py", line 287, in <module>
    import pickle
  File "/home/vstinner/prog/python/master/Lib/pickle.py", line 39, in <module>
    from _pickle import PickleBuffer
ModuleNotFoundError: import of _pickle halted; None in sys.modules

It is no longer possible to benchmark the pure Python implement of pickle.

--pure-python uses this code path:

def is_module_accelerated(module):
    return getattr(pickle.Pickler, '__module__', '<jython>') == 'pickle'

(...)

        if six.PY3:
            sys.modules['_pickle'] = None
        import pickle
        if not is_module_accelerated(pickle):
            raise RuntimeError("Unexpected C accelerators for pickle")

Should I remove the benchmark?

I wrote a simple PR, PR 14016, to fix the pure Python implementation of pickle: simply restrict pickle to protocol 4 if PickleBuffer is missing.

New changeset 63ab4ba by Victor Stinner in branch 'master':
bpo-37210: Fix pure Python pickle when _pickle is unavailable (GH-14016)
63ab4ba

New changeset cbda40d by Miss Islington (bot) in branch '3.8':
bpo-37210: Fix pure Python pickle when _pickle is unavailable (GH-14016)
cbda40d

I pushed my change to 3.8 and master branches. I close the issue.