Skip to content

gh-105250: Fix NEWOBJ handling of custom metaclasses in C pickle - #155920

Open
chaerrypick01 wants to merge 4 commits into
python:mainfrom
chaerrypick01:fix-gh-105250-pickle-newobj-metaclass
Open

gh-105250: Fix NEWOBJ handling of custom metaclasses in C pickle#155920
chaerrypick01 wants to merge 4 commits into
python:mainfrom
chaerrypick01:fix-gh-105250-pickle-newobj-metaclass

Conversation

@chaerrypick01

@chaerrypick01 chaerrypick01 commented Aug 17, 2026

Copy link
Copy Markdown

The NEWOBJ and NEWOBJ_EX opcodes are documented to call cls.__new__(cls, *args), but the C implementation called tp_new directly, so a metaclass __getattribute__ hook was skipped unless the class happened to define __new__ in Python. Perform a real attribute lookup of cls.__new__ when the class has a custom metaclass, matching the pure Python implementation.

When the metaclass is exactly type, no hook can exist and the lookup is not observable, so tp_new is still called directly - no performance change for ordinary classes.

The new test runs against both implementations and covers NEWOBJ (protocols 2-5) and NEWOBJ_EX (protocols 4-5); the cases inheriting object.__new__ fail on the C unpickler without this change.

I benchmarked the three affected paths with pyperf 2.10.0 (script below). Both interpreters were built with the same plain ./configure (release build, Py_DEBUG=0). Baseline is main at 70fdc96. Each benchmark unpickles a
list of 1000 instances.

Benchmark base patched
newobj_default (NEWOBJ, default metaclass) 295 µs 296 µs: not significant
newobj_ex_default (NEWOBJ_EX, default metaclass) 604 µs 605 µs: not significant
newobj_metaclass (NEWOBJ, custom metaclass) 293 µs 338 µs: 1.15x slower

The fast path shows no measurable regression for either opcode — the added Py_TYPE(cls) == &PyType_Type check is lost in the noise. The 1.15x slowdown is confined to the new slow path, which is only taken for classes with a custom metaclass: the cost of the PyObject_GetAttr(cls, '__new__') call plus building the argument tuple, on the path where the previous behaviour did not match the documented semantics.

Environment: macOS 26.5.1, Apple M3 Pro, no CPU isolation (pyperf system tune is Linux-only), 20 processes per benchmark.

Given the trade-off — documented cls.__new__ semantics for custom-metaclass classes, at ~15% on that path and no change for ordinary classes — I'd like your feedback on whether this is acceptable, or whether you'd prefer to close
gh-105250 as a known limitation instead.

Benchmark script
"""Micro-benchmark for load_newobj() in Modules/_pickle.c (gh-105250)."""
import pickle
import pyperf

assert pickle.Unpickler.__module__ == '_pickle', 'need the C implementation'


class Point:
    def __init__(self, x, y):
        self.x = x
        self.y = y


class PointEx:
    def __init__(self, x, y):
        self.x = x
        self.y = y

    def __getnewargs_ex__(self):
        return (self.x,), {'y': self.y}

    def __new__(cls, x=0, y=0):
        return super().__new__(cls)


class Meta(type):
    pass


class PointMeta(metaclass=Meta):
    def __init__(self, x, y):
        self.x = x
        self.y = y


N = 1000
DATA_DEFAULT = pickle.dumps([Point(i, i) for i in range(N)], protocol=2)
DATA_EX = pickle.dumps([PointEx(i, i) for i in range(N)], protocol=4)
DATA_META = pickle.dumps([PointMeta(i, i) for i in range(N)], protocol=2)

runner = pyperf.Runner()
runner.metadata['description'] = 'unpickle NEWOBJ/NEWOBJ_EX (gh-105250)'
runner.bench_func('newobj_default', pickle.loads, DATA_DEFAULT)
runner.bench_func('newobj_ex_default', pickle.loads, DATA_EX)
runner.bench_func('newobj_metaclass', pickle.loads, DATA_META)

The NEWOBJ and NEWOBJ_EX opcodes are documented to call
cls.__new__(cls, *args), but the C implementation called tp_new
directly, so a metaclass __getattribute__ hook was skipped unless the
class happened to define __new__ in Python.  Perform a real attribute
lookup of cls.__new__ when the class has a custom metaclass, matching
the pure Python implementation.  The default-metaclass case keeps
calling tp_new directly, where the lookup is not observable.
@python-cla-bot

python-cla-bot Bot commented Aug 17, 2026

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

Comment thread Misc/NEWS.d/next/Library/2026-08-17-11-35-48.gh-issue-105250.wNaXOE.rst Outdated
Comment thread Lib/test/picklecommon.py
metaclass_new_lookups = []

class LookupLoggingMeta(type):
def __getattribute__(cls, name):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to move look up count as class varaible
Something like:

class LookupLoggingMeta(type):
     new_lookup_count = 0
    def __getattribute__(cls, name):

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in f22a1a5 — moved the count to a class variable on the metaclass.

@corona10

Copy link
Copy Markdown
Member

Can you also provide benchmark compare to main branch by using pyperf?

Comment thread Lib/test/picklecommon.py Outdated

def __getattribute__(cls, name):
if name == '__new__':
LookupLoggingMeta.new_lookup_count += 1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
LookupLoggingMeta.new_lookup_count += 1
cls.new_lookup_count += 1

?

Comment thread Modules/_pickle.c
Comment on lines +6339 to +6358
/* Look __new__ up on the class so that a custom metaclass
__getattribute__ observes the lookup, as in the Python
implementation. */
PyObject *func = PyObject_GetAttr(cls, &_Py_ID(__new__));
if (func == NULL) {
goto error;
}
Py_ssize_t nargs = PyTuple_GET_SIZE(args);
PyObject *newargs = PyTuple_New(nargs + 1);
if (newargs == NULL) {
Py_DECREF(func);
goto error;
}
PyTuple_SET_ITEM(newargs, 0, Py_NewRef(cls));
for (Py_ssize_t i = 0; i < nargs; i++) {
PyTuple_SET_ITEM(newargs, i + 1,
Py_NewRef(PyTuple_GET_ITEM(args, i)));
}
obj = PyObject_Call(func, newargs, kwargs);
Py_DECREF(newargs);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/* Look __new__ up on the class so that a custom metaclass
__getattribute__ observes the lookup, as in the Python
implementation. */
PyObject *func = PyObject_GetAttr(cls, &_Py_ID(__new__));
if (func == NULL) {
goto error;
}
Py_ssize_t nargs = PyTuple_GET_SIZE(args);
PyObject *newargs = PyTuple_New(nargs + 1);
if (newargs == NULL) {
Py_DECREF(func);
goto error;
}
PyTuple_SET_ITEM(newargs, 0, Py_NewRef(cls));
for (Py_ssize_t i = 0; i < nargs; i++) {
PyTuple_SET_ITEM(newargs, i + 1,
Py_NewRef(PyTuple_GET_ITEM(args, i)));
}
obj = PyObject_Call(func, newargs, kwargs);
Py_DECREF(newargs);
PyThreadState *tstate = _PyThreadState_GET();
obj = _PyObject_Call_Prepend(tstate, func, cls, args, kwargs);

Can you benchmark this one too?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

2 participants