Pure-Python pickle._Pickler can be significantly optimized

[mjbommar]

Feature or enhancement

Proposal:

Inspired by some of the optimization patterns used in #148700 , I took a look at the pure Python pickler and found some low-hanging fruit. Some of these changes do increase the complexity of maintaining the code, but the double-digit performance gains might be worth it given dill and cloudpickle usage patterns.

FYI, my testing showed 19-37% speed increase for dill so far.

Below is Opus's summary of the things kept in the PR I will be submitting. The PR includes microbenchmark experiments (Misc/pickle-perf-diary.md) for a much broader set of changes that did not survive.

  The pure-Python pickle._Pickler (Lib/pickle.py) is used as the fallback when
  the _pickle C accelerator is unavailable and by code that explicitly
  subclasses pickle.Pickler in Python. Its save() hot path currently does
  more work than necessary relative to Modules/_pickle.c::save():

  - the memo.get(id(obj)) and reducer_override probes run before the atomic-
    type dispatch, so int / None / bool / float values pay for checks the
    C reference implementation skips for these types;
  - _batch_appends and _batch_setitems use itertools.batched() + enumerate()
    even for exact list / dict instances, where the C accelerator's
    batch_list_exact / batch_dict_exact skip that machinery;
  - memoize() dispatches through put() to write a single MEMOIZE byte for
    protocol >= 4 (the common case);
  - save_long rebuilds the 2-byte BININT1 opcode via struct.pack on every
    small-int save;
  - framer.commit_frame() is invoked as a full method call per save() even
    though the hot check is a single length compare;
  - bytes values aren't in the save() dispatch fast path alongside str.

  Attached patch addresses each of these with measured wins of -20% to -49%
  on pure-Python _Pickler dump across a representative workload set
  (list_of_ints, list_of_strs, dict of str->int, deep list, nested list of
  dicts, list of short bytes, dict of bytes->int). Full CPython test suite,
  dill 0.4.1, and cloudpickle 3.1.2 all pass / match baseline. Detailed
  experiment ledger with raw bench data committed under
  Misc/pickle-perf-diary.md and Misc/pickle-perf-data/ on the proposed
  branch.

There is notable user-facing change that can be addressed but I'd defer to reviewers on how important it is: atomic types no longer invoke reducer_override, wich actually aligns this with _pickle.c::save().

Has this already been discussed elsewhere?

No response given

Links to previous discussion of this feature:

No response

Linked PRs

• gh-148706: Pure-Python pickle._Pickler can be significantly optimized #148707

[serhiy-storchaka]

• memoize() dispatches through put() to write a single MEMOIZE byte for
  protocol >= 4 (the common case);

There is even simpler and faster trick. Add the following lines in the constructor:

if self.proto >= 4 and not self.fast:
    self.memoize = lambda obj: self.write(MEMOIZE)

But this will break code that uses Pickler in unusual way and change proto or fast after construction and code that overrides memoize or put in subclasses of Pickler.

• bytes values aren't in the save() dispatch fast path alongside str.

What do you mean? What fast path?

It seems that your PR inlines some methods, which breaks the code that relies on possibility to override these methods, and adds some special cases which significantly increase the volume of code and makes it less maintainable. I am not sure that it is worth it, because the wast majority of users uses the C implementation.

What are the issues with using the C implementation in dill? Would not be better to solve these issues?

[mjbommar]

What are the issues with using the C implementation in dill? Would not be better to solve these issues?

There are many different reasons, but the biggest I've personally encountered have to do with lambdas or dynamically generated classes, which are very common in places like celery, dask, scikit-learn, etc.

For context, dill and cloudpickle regularly have >10M downloads per day, so anything we could do in pickle._Pickler would have big user impact:

https://pypistats.org/packages/cloudpickle
https://pypistats.org/packages/dill

I agree that this does not improve code maintainability, but looking over the horizon into the longer term, LLM assistance is changing that calculus anyway.

[picnixz]

I agree that this does not improve code maintainability, but looking over the horizon into the longer term, LLM assistance is changing that calculus anyway.

This is not an argument we are willing to accept though. Some may not want to use LLMs, including maintainera, and we have a policy against LLM-generated PRs.

I do not understand why the C implementation of pickle cannot be used by dill and cloudpickle.

[mjbommar]

I do not understand why the C implementation of pickle cannot be used by dill and cloudpickle.

uqfoundation/dill#485 does a much better job of showing the reasons and why.

I won't pretend to know all the reasons, but there were over 6.5M downloads yesterday for dill.

[picnixz]

I do not think it is important to restate the number of downloads. You already did once and while it is an argument about the popularity of the library, the dill's issue has been opened for 4 years. I think the maintenance cost would shift from them to us and I do not necessarily see why it should really matter. AFAICT, a custom solution for dill seems better so I am going to close this. If dill maintainers want to support faster pure Python pickle they should check if their concerns are still valid.