Make function piclkling deterministic by default

[ogrisel]

Alternative to #418 that does not need sorting. I ripped off some of the test code of @effigies. If we agree this is good idea, I will amend my commit to grant co-authorship before merging.

[codecov]

Codecov Report

Merging #428 (5ac1728) into master (2a79b69) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #428      +/-   ##
==========================================
+ Coverage   91.77%   91.80%   +0.02%     
==========================================
  Files           4        4              
  Lines         669      671       +2     
  Branches      136      137       +1     
==========================================
+ Hits          614      616       +2     
  Misses         34       34              
  Partials       21       21              

Impacted Files	Coverage Δ
cloudpickle/cloudpickle.py	86.95% <100.00%> (ø)
cloudpickle/cloudpickle_fast.py	96.78% <100.00%> (+0.02%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2a79b69...5ac1728. Read the comment docs.

[ogrisel]

For some reason it does not work with the CPython 3.10 dev branch on Windows...

[ogrisel]

On Windows with CPython 3.10, on the first run, I get:

...
  226: \x8c         SHORT_BINUNICODE 'func_with_globals'
  245: \x94         MEMOIZE    (as 15)
  246: M            BININT2    2346
  249: C            SHORT_BINBYTES b'\x00\x7f\x00\n'
  255: \x94         MEMOIZE    (as 16)
  256: )            EMPTY_TUPLE
  257: )            EMPTY_TUPLE
  258: t            TUPLE      (MARK at 90)
...

while on the second run:

...
  226: \x8c         SHORT_BINUNICODE 'func_with_globals'
  245: \x94         MEMOIZE    (as 15)
  246: M            BININT2    2346
  249: C            SHORT_BINBYTES b'\x00B'
  253: \x94         MEMOIZE    (as 16)
  254: )            EMPTY_TUPLE
  255: )            EMPTY_TUPLE
  256: t            TUPLE      (MARK at 90)
...

So the difference is the payload of the SHORT_BINBYTES op at byte offset 249...

[ogrisel]

For reference on Python 3.8 and 3.9 (on macos) I always get the following lines for the same section:

...
  229: \x8c         SHORT_BINUNICODE 'func_with_globals'
  248: \x94         MEMOIZE    (as 15)
  249: M            BININT2    2346
  252: C            SHORT_BINBYTES b'\x00\x01'
  256: \x94         MEMOIZE    (as 16)
  257: )            EMPTY_TUPLE
  258: )            EMPTY_TUPLE
  259: t            TUPLE      (MARK at 90)
...

I do not understand what those few bytes refer to though.

[ogrisel]

By looking at the source code of cloudpickle_fast._code_reduce, it seems that this field would be co_lnotab field of the code object.

So now we need to understand why it's not stable on Windows in CPython 3.10. Maybe @pitrou has an idea?

[ogrisel]

By googling I found about the following:

https://www.python.org/dev/peps/pep-0626/#the-co-lnotab-attribute

It seems that this attribute is deprecated. I don't understand everything though. @pierreglaser you might have a better idea :)

[pitrou]

It seems you should use co_linetable instead?

[ogrisel]

Indeed, this is what I as led to believe although there is no documentation for the CodeType constructor parameters: https://docs.python.org/3.10/library/types.html?highlight=codetype#types.CodeType . I am building CPython 3.10 locally to investigate.

[ogrisel]

The CPython test suite is still testing the constructor by passing co_lnotab:

https://github.com/python/cpython/blob/main/Lib/test/test_code.py#L212-L228

[pitrou]

Perhaps any bytes object is accepted by the constructor? :-)

[ogrisel]

Yes, apparently passing an empty bytes object works (the cloudpickle tests pass). But I am not sure what could be the consequences, e.g. for debuggers.

For reference, the CodeType signature still reference the deprecated field, e.g. using IPython introspection:

>>> import types
>>> types.CodeType?
Init signature: types.CodeType(self, /, *args, **kwargs)
Docstring:     
code(argcount, posonlyargcount, kwonlyargcount, nlocals, stacksize,
      flags, codestring, constants, names, varnames, filename, name,
      firstlineno, lnotab[, freevars[, cellvars]])

Create a code object.  Not for the faint of heart.
Type:           type
Subclasses:

I will open an issue on the CPython tracker to ask about the new recommended way to use the CodeType constructor.

[ogrisel]

Maybe I spoke too quickly, the IPython session above was running 3.9. Let me try to use co_linetable instead.

[ogrisel]

Ok that did it. We will have the same kind of update to do for Python 3.11 because of co_exceptiontable but I would rather wait we start officially CI support for 3.11 with the first alpha in a few months.

[ogrisel]

Merged, thanks @effigies for the initial report and the tests and @pitrou for the help :)