Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Faster default __reduce__ for classes without __init__ #67607

Closed
serhiy-storchaka opened this issue Feb 9, 2015 · 4 comments
Closed

Faster default __reduce__ for classes without __init__ #67607

serhiy-storchaka opened this issue Feb 9, 2015 · 4 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage

Comments

@serhiy-storchaka
Copy link
Member

BPO 23419
Nosy @amauryfa, @pitrou, @avassalotti, @serhiy-storchaka
Files
  • object_reduce_no_init.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/serhiy-storchaka'
    closed_at = <Date 2015-03-20.16:41:30.434>
    created_at = <Date 2015-02-09.11:00:44.020>
    labels = ['interpreter-core', 'performance']
    title = 'Faster default __reduce__ for classes without __init__'
    updated_at = <Date 2015-03-20.16:41:30.433>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2015-03-20.16:41:30.433>
    actor = 'serhiy.storchaka'
    assignee = 'serhiy.storchaka'
    closed = True
    closed_date = <Date 2015-03-20.16:41:30.434>
    closer = 'serhiy.storchaka'
    components = ['Interpreter Core']
    creation = <Date 2015-02-09.11:00:44.020>
    creator = 'serhiy.storchaka'
    dependencies = []
    files = ['38052']
    hgrepos = []
    issue_num = 23419
    keywords = ['patch']
    message_count = 4.0
    messages = ['235600', '238499', '238500', '238701']
    nosy_count = 4.0
    nosy_names = ['amaury.forgeotdarc', 'pitrou', 'alexandre.vassalotti', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'rejected'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'performance'
    url = 'https://bugs.python.org/issue23419'
    versions = ['Python 3.5']

    @serhiy-storchaka
    Copy link
    Member Author

    Proposed patch makes faster default __reduce__ implementation for the case when there is no non-trivial __init__ defined (e.g. for named tuples). In this case __reduce__ will return (cls, newargs) instead of (copyreg.__newobj__, (cls,) + newargs).

    >>> pickletools.dis(pickletools.optimize(pickle.dumps(turtle.Vec2D(12, 34), 3)))
    Before:
        0: \x80 PROTO      3
        2: c    GLOBAL     'turtle Vec2D'
       16: K    BININT1    12
       18: K    BININT1    34
       20: \x86 TUPLE2
       21: \x81 NEWOBJ
       22: .    STOP
    After:
        0: \x80 PROTO      3
        2: c    GLOBAL     'turtle Vec2D'
       16: K    BININT1    12
       18: K    BININT1    34
       20: \x86 TUPLE2
       21: R    REDUCE
       22: .    STOP

    Pickled size is the same, but pickling is faster. The benefit is in avoiding of importing copyreg.__newobj__ and allocating new tuple (cls,) + newargs.

    Microbenchmarks results:

    $ ./python -m timeit -s "import pickle; from turtle import Vec2D; a = [Vec2D(i, i+0.1) for i in range(1000)]" -- "pickle.dumps(a)"

    Before: 100 loops, best of 3: 16.3 msec per loop
    After: 100 loops, best of 3: 15.2 msec per loop

    $ ./python -m timeit -s "import copy; from turtle import Vec2D; a = [Vec2D(i, i+0.1) for i in range(1000)]" -- "copy.deepcopy(a)"

    Before: 10 loops, best of 3: 96.6 msec per loop
    After: 10 loops, best of 3: 88.7 msec per loop

    @serhiy-storchaka serhiy-storchaka added interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage labels Feb 9, 2015
    @amauryfa
    Copy link
    Member

    But isn't the result different?
    NEWOBJ calls cls.__new__() and __init__ is skipped.
    REDUCE calls cls(): both __new__ and __init__ are used.

    @amauryfa
    Copy link
    Member

    Sorry, I missed the important point:
    "for classes without __init__"

    @serhiy-storchaka
    Copy link
    Member Author

    For a namedtuple such as turtle.Vec2D there is no significant difference in the time of unpickling. But for simpler type it is.

    $ ./python -m timeit -s 'import pickle, turtle; global I' -s 'class I(int): pass' -s 'p = pickle.dumps([I(i) for i in range(1000)], 3)' -- 'pickle.loads(p)'

    Before: 1000 loops, best of 3: 1.6 msec per loop
    After: 1000 loops, best of 3: 1.82 msec per loop

    So I withdraw my patch. Unpickling performance is more important than pickling performance, and status quo wins. Sorry for the noise.

    @serhiy-storchaka serhiy-storchaka self-assigned this Mar 20, 2015
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants