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

Simplify PyImport_ImportModuleLevelObject: avoid temporary tuple of str.partition/rpartition #71135

Closed
vstinner opened this issue May 4, 2016 · 4 comments
Labels
performance Performance or resource usage

Comments

@vstinner
Copy link
Member

vstinner commented May 4, 2016

BPO 26948
Nosy @brettcannon, @ncoghlan, @vstinner, @ericsnowcurrently, @serhiy-storchaka
Files
  • import.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 = None
    closed_at = <Date 2016-05-20.09:38:13.599>
    created_at = <Date 2016-05-04.12:48:16.966>
    labels = ['performance']
    title = 'Simplify PyImport_ImportModuleLevelObject: avoid temporary tuple of str.partition/rpartition'
    updated_at = <Date 2016-05-20.09:38:13.598>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2016-05-20.09:38:13.598>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2016-05-20.09:38:13.599>
    closer = 'vstinner'
    components = []
    creation = <Date 2016-05-04.12:48:16.966>
    creator = 'vstinner'
    dependencies = []
    files = ['42714']
    hgrepos = []
    issue_num = 26948
    keywords = ['patch']
    message_count = 4.0
    messages = ['264809', '264814', '264871', '265923']
    nosy_count = 5.0
    nosy_names = ['brett.cannon', 'ncoghlan', 'vstinner', 'eric.snow', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = 'performance'
    url = 'https://bugs.python.org/issue26948'
    versions = ['Python 3.6']

    @vstinner
    Copy link
    Member Author

    vstinner commented May 4, 2016

    Attached patch simplifies PyImport_ImportModuleLevelObject to avoid the temporary tuple created by str.partition/rpartition(). I don't expect any difference on the performance, it's more to cleanup the code.

    @vstinner vstinner added the performance Performance or resource usage label May 4, 2016
    @serhiy-storchaka
    Copy link
    Member

    LGTM, but I left a couple of minor suggestions on Rietveld. I hope they can make the code yet cleaner.

    @ncoghlan
    Copy link
    Contributor

    ncoghlan commented May 5, 2016

    No objections from me (and I don't have anything to add to Serhiy's comments on the code review)

    @vstinner
    Copy link
    Member Author

    I modified my patch to address Serhiy's comments. Thanks for the your review Serhiy.

    changeset: 101442:779563dd701c
    tag: tip
    user: Victor Stinner <victor.stinner@gmail.com>
    date: Fri May 20 11:36:13 2016 +0200
    files: Python/import.c
    description:
    Cleanup import.c

    • Replace PyUnicode_RPartition() with PyUnicode_FindChar() and
      PyUnicode_Substring() to avoid the creation of a temporary tuple.
    • Use PyUnicode_FromFormat() to build a string and avoid the single_dot ('.')
      singleton

    Thanks Serhiy Storchaka for your review.

    @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
    performance Performance or resource usage
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants