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

Jump to bottom

Custom iterator to memoryview - performance improvement #85898

Closed
dxflores mannequin opened this issue Sep 6, 2020 · 6 comments
Closed

Custom iterator to memoryview - performance improvement #85898

dxflores mannequin opened this issue Sep 6, 2020 · 6 comments
Labels
3.10 only security fixes

Comments

@dxflores
Copy link
Mannequin

dxflores mannequin commented Sep 6, 2020

BPO 41732
Nosy @gvanrossum, @vstinner, @corona10, @dxflores
PRs
  • bpo-41732: add iterator to memoryview #22119
    		•

    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 = None
closed_at = <Date 2020-09-08.09:19:26.875>
created_at = <Date 2020-09-06.09:06:52.941>
labels = ['3.10']
title = 'Custom iterator to memoryview - performance improvement'
updated_at = <Date 2020-09-08.10:11:31.178>
user = 'https://github.com/dxflores'

    bugs.python.org fields:

    activity = <Date 2020-09-08.10:11:31.178>
actor = 'dxflores'
assignee = 'none'
closed = True
closed_date = <Date 2020-09-08.09:19:26.875>
closer = 'corona10'
components = []
creation = <Date 2020-09-06.09:06:52.941>
creator = 'dxflores'
dependencies = []
files = []
hgrepos = []
issue_num = 41732
keywords = ['patch']
message_count = 6.0
messages = ['376460', '376549', '376550', '376558', '376559', '376561']
nosy_count = 4.0
nosy_names = ['gvanrossum', 'vstinner', 'corona10', 'dxflores']
pr_nums = ['22119']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue41732'
versions = ['Python 3.10']
    @dxflores
    Copy link
    Mannequin Author

    dxflores mannequin commented Sep 6, 2020

    Hi,

    Here's a patch that adds an iterator to memoryview, which makes iterating over memoryview objs considerably faster.

    The following values result from a compilation with debug ON.

    ------------------------------------------------------------------------

    Without patch:

    cpython: ./python.exe -m timeit -s "a=memoryview(b'x'*1000000)" "for x in a: pass"
    5 loops, best of 5: 98 msec per loop

    cpython: ./python.exe -m timeit -s "a=b'x'*1000000" "for x in a: pass"
    5 loops, best of 5: 68.6 msec per loop

    With patch:

    cpython: ./python.exe -m timeit -s "a=memoryview(b'x'*1000000)" "for x in a: pass"
    5 loops, best of 5: 68.1 msec per loop

    cpython: ./python.exe -m timeit -s "a=b'x'*1000000" "for x in a: pass"
    5 loops, best of 5: 70 msec per loop

    ------------------------------------------------------------------------

    Please let me know your thoughts regarding it.

    Diogo

    @dxflores dxflores mannequin added 3.10 only security fixes labels Sep 6, 2020
    @corona10
    Copy link
    Member

    corona10 commented Sep 8, 2020

    New changeset b0ac5d7 by dxflores in branch 'master':
    bpo-41732: add iterator to memoryview (GH-22119)
    b0ac5d7

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Sep 8, 2020

    It would have been nice to have had a chance to look at this, but
    apparently those days are over.

    @corona10
    Copy link
    Member

    corona10 commented Sep 8, 2020

    Thanks, Diogo for working on this issue.
    And thank you for the mentoring guido!

    @corona10 corona10 closed this as completed Sep 8, 2020
    @corona10 corona10 closed this as completed Sep 8, 2020
    @vstinner
    Copy link
    Member

    vstinner commented Sep 8, 2020

    Thanks Diogo Flores, it's a nice optimization!

    Stefan Krah: "It would have been nice to have had a chance to look at this, but apparently those days are over."

    The PR has been approved by two core developers. It's a nice optimization, I don't see what is the problem here.

    @dxflores
    Copy link
    Mannequin Author

    dxflores mannequin commented Sep 8, 2020

    Thank you Dong-hee Na and Victor.

    @Stefan Krah: I am sorry that you didn't had the chace to try out the patch before it got merged, however iff you are curious about the any aspect of the implementation (or the motivation behind it) I am happy to discuss it in depth.

    @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
    Assignees
    No one assigned
    Labels
    3.10 only security fixes
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @vstinner @corona10