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

Verify the etree_parse and etree_iterparse benchmarks are working appropriately #69824

Closed
brettcannon opened this issue Nov 16, 2015 · 20 comments
Labels
expert-XML extension-modules C modules in the Modules dir performance Performance or resource usage stdlib Python modules in the Lib dir

Comments

@brettcannon
Copy link
Member

brettcannon commented Nov 16, 2015

BPO 25638
Nosy @brettcannon, @pitrou, @scoder, @serhiy-storchaka
PRs
  • [WIP] bpo-35502: Fix memory leak in xml.etree iterparse() #11169
  • Dependencies
  • bpo-25814: Propagate all errors from ElementTree.iterparse
  • Files
  • etree_iterparse.patch
  • etree_iterparse_2.patch
  • etree_start_handler_no_attrib.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-03-12.17:21:48.895>
    created_at = <Date 2015-11-16.19:41:23.215>
    labels = ['extension-modules', 'expert-XML', 'library', 'invalid', 'performance']
    title = 'Verify the etree_parse and etree_iterparse benchmarks are working appropriately'
    updated_at = <Date 2018-12-14.22:10:01.246>
    user = 'https://github.com/brettcannon'

    bugs.python.org fields:

    activity = <Date 2018-12-14.22:10:01.246>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2016-03-12.17:21:48.895>
    closer = 'brett.cannon'
    components = ['Extension Modules', 'Library (Lib)', 'XML', 'Benchmarks']
    creation = <Date 2015-11-16.19:41:23.215>
    creator = 'brett.cannon'
    dependencies = ['25814']
    files = ['41110', '41167', '41276']
    hgrepos = []
    issue_num = 25638
    keywords = ['patch']
    message_count = 20.0
    messages = ['254746', '254748', '254751', '254752', '254753', '254763', '254764', '255021', '255024', '255027', '255050', '255394', '255936', '256013', '256039', '256059', '256158', '256167', '256172', '261651']
    nosy_count = 6.0
    nosy_names = ['brett.cannon', 'pitrou', 'scoder', 'eli.bendersky', 'python-dev', 'serhiy.storchaka']
    pr_nums = ['11169']
    priority = 'normal'
    resolution = 'not a bug'
    stage = None
    status = 'closed'
    superseder = None
    type = 'performance'
    url = 'https://bugs.python.org/issue25638'
    versions = ['Python 3.6']

    @brettcannon
    Copy link
    Member Author

    brettcannon commented Nov 16, 2015

    If you look at bit.ly/pycon-ca-keynote you will notice that the etree_parse and etree_iterparse benchmarks were horrible for everyone. Because of how badly everyone seemed to do, I think the benchmarks should be verified to be doing reasonable things on implementations other than CPython 2.7.

    @brettcannon brettcannon added the performance Performance or resource usage label Nov 16, 2015
    @brettcannon brettcannon self-assigned this Nov 16, 2015
    @pitrou
    Copy link
    Member

    pitrou commented Nov 16, 2015

    Would you have a quick summary for those not willing to watch a whole keynote?

    @brettcannon
    Copy link
    Member Author

    brettcannon commented Nov 16, 2015

    That link is to a Jupyter notebook so you don't have to watch anything. Plus the video is not even up yet so you can't skip the keynote even if you wanted to since you can't watch it yet. :)

    @pitrou
    Copy link
    Member

    pitrou commented Nov 16, 2015

    Ok, so when you say "horrible for everyone", this is really IronPython and Jython, right? :-) Other runtimes seem to do ok (perhaps not stellar, but ok).

    @brettcannon
    Copy link
    Member Author

    brettcannon commented Nov 16, 2015

    Well, Jython and IronPython obviously did the worst, but even Python 3 didn't do as well as I would have expected, so I still want to double-check the benchmarks to see if it's obvious why CPython 2.7 beats out everyone.

    @serhiy-storchaka
    Copy link
    Member

    serhiy-storchaka commented Nov 16, 2015

    I think these histograms would look better with logarithmic scale.

    @brettcannon
    Copy link
    Member Author

    brettcannon commented Nov 16, 2015

    Let's not pollute the issue with a critique of my notebook. You can feel free to email me personally to discuss it if you want, including why I purposefully didn't use a logarithmic scale.

    @serhiy-storchaka
    Copy link
    Member

    serhiy-storchaka commented Nov 20, 2015

    Sorry Brett.

    How tests were ran? There are two implementations of ElementTree, accelerated and non-accelerated. xml.etree.ElementTree by default is accelerated in Python 3, but non-accelerated in Python 2.

    $ python2.7 bm_elementtree.py -n 7 --take_geo_mean 
    0.463665158795
    $ python2.7 bm_elementtree.py -n 7 --take_geo_mean --etree-module=xml.etree.ElementTree
    5.46309932568
    $ python3.4 bm_elementtree.py -n 7 --take_geo_mean --etree-module=xml.etree.ElementTree
    0.813397633467649
    $ python3.4 bm_elementtree.py -n 7 --take_geo_mean --etree-module=xml.etree.ElementTree --no-accelerator
    5.31174765817514

    If run the test with the same options --etree-module=xml.etree.ElementTree, it will use accelerated implementation in Python 3 and non-accelerated in Python 2.

    @brettcannon
    Copy link
    Member Author

    brettcannon commented Nov 20, 2015

    The commands I used are in the notebook for each implementation and you can get the same result with python3 perf.py -b etree python2 python3.

    @serhiy-storchaka
    Copy link
    Member

    serhiy-storchaka commented Nov 20, 2015

    The slowing down Python 3 can be related to adding XMLPullParser (bpo-17741).

    @serhiy-storchaka
    Copy link
    Member

    serhiy-storchaka commented Nov 21, 2015

    Proposed patch optimizes iterparse(). Now it is only 33% slower than in 2.7 (was 2.6 times slower).

    @serhiy-storchaka serhiy-storchaka added extension-modules C modules in the Modules dir stdlib Python modules in the Lib dir expert-XML performance Performance or resource usage labels Nov 21, 2015
    @serhiy-storchaka
    Copy link
    Member

    serhiy-storchaka commented Nov 26, 2015

    Updated to tip.

    @brettcannon
    Copy link
    Member Author

    brettcannon commented Dec 5, 2015

    Serhiy's latest patch LGTM.

    @serhiy-storchaka
    Copy link
    Member

    serhiy-storchaka commented Dec 6, 2015

    Thank you for your review Brett. First than apply this optimization I want to fix errors propagating issue (bpo-25814). The patch for it is mainly the simplified part of the patch for this issue.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Dec 7, 2015

    New changeset dd67c8c53aea by Serhiy Storchaka in branch 'default':
    Issue bpo-25638: Optimized ElementTree.iterparse(); it is now 2x faster.
    https://hg.python.org/cpython/rev/dd67c8c53aea

    @serhiy-storchaka
    Copy link
    Member

    serhiy-storchaka commented Dec 7, 2015

    The iterparse benchmark in 3.6 still is 30% slower than in 2.7. The parse benchmark is 70% slower. Hence there are other causes of the slowing down.

    One of causes is that in 3.x an empty dict instead of None is passed to start handler as attrib parameter if the start tag has no attributes. This makes parsing parsing about 10% slower.

    @serhiy-storchaka
    Copy link
    Member

    serhiy-storchaka commented Dec 9, 2015

    Following patch speeds up ElementTree parsing (the result of the etree parse benchmark is improved by 10%). Actually it restores 2.7 code and avoids creating an empty dict for attributes if not needed.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Dec 10, 2015

    New changeset 1fe904420c20 by Serhiy Storchaka in branch 'default':
    Issue bpo-25638: Optimized ElementTree parsing; it is now 10% faster.
    https://hg.python.org/cpython/rev/1fe904420c20

    @serhiy-storchaka
    Copy link
    Member

    serhiy-storchaka commented Dec 10, 2015

    Thank you for your review Brett. Now the parse benchmark in 3.6 is only 50% slower than in 2.7. Will continue to find bottlenecks.

    @serhiy-storchaka
    Copy link
    Member

    serhiy-storchaka commented Mar 12, 2016

    I am not able to find the cause of the slowdown.

    I think this issue can be closed now. The etree_parse and etree_iterparse benchmarks are working appropriately and showing real regression in CPython 3.x. The cause of the regression is not known.

    @serhiy-storchaka serhiy-storchaka removed their assignment Mar 12, 2016
    @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
    expert-XML extension-modules C modules in the Modules dir performance Performance or resource usage stdlib Python modules in the Lib dir
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants