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

Lazy GC tracking frame #73235

Closed
methane opened this issue Dec 22, 2016 · 15 comments
Closed

Lazy GC tracking frame #73235

methane opened this issue Dec 22, 2016 · 15 comments
Labels
performance Performance or resource usage

Comments

@methane
Copy link
Member

methane commented Dec 22, 2016

BPO 29049
Nosy @vstinner, @methane, @serhiy-storchaka, @MojoVampire, @zhangyangyu
PRs
  • [Do Not Merge] Convert Misc/NEWS so that it is managed by towncrier #552
  • Files
  • frame-lazy-track.patch
  • patched.json.gz
  • default.json.gz
  • 29049-fix-generator-refleak.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-12-26.11:19:07.771>
    created_at = <Date 2016-12-22.12:00:44.795>
    labels = ['performance']
    title = 'Lazy GC tracking frame'
    updated_at = <Date 2017-09-18.08:58:51.397>
    user = 'https://github.com/methane'

    bugs.python.org fields:

    activity = <Date 2017-09-18.08:58:51.397>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2016-12-26.11:19:07.771>
    closer = 'methane'
    components = []
    creation = <Date 2016-12-22.12:00:44.795>
    creator = 'methane'
    dependencies = []
    files = ['45997', '45999', '46000', '46038']
    hgrepos = []
    issue_num = 29049
    keywords = ['patch']
    message_count = 15.0
    messages = ['283830', '283844', '283941', '283942', '283975', '283977', '283988', '284007', '284012', '284013', '284014', '284016', '284017', '284019', '302429']
    nosy_count = 6.0
    nosy_names = ['vstinner', 'methane', 'python-dev', 'serhiy.storchaka', 'josh.r', 'xiang.zhang']
    pr_nums = ['552']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'performance'
    url = 'https://bugs.python.org/issue29049'
    versions = []

    @methane
    Copy link
    Member Author

    methane commented Dec 22, 2016

    Don't _PyObject_GC_TRACK(frame) before using it.
    After evaluation is finished, do tracking only if refcnt is not one.

    result without --enable-optimization:
    $ ~/local/cpython/bin/pyperf compare_to default.json.gz patched.json.gz -G
    Slower (15):

    • scimark_sparse_mat_mult: 5.83 ms +- 0.03 ms -> 6.12 ms +- 0.35 ms: 1.05x slower (+5%)
    • sympy_expand: 734 ms +- 21 ms -> 765 ms +- 40 ms: 1.04x slower (+4%)
    • sympy_str: 327 ms +- 10 ms -> 339 ms +- 13 ms: 1.04x slower (+4%)
    • scimark_fft: 498 ms +- 2 ms -> 512 ms +- 2 ms: 1.03x slower (+3%)
    • scimark_monte_carlo: 172 ms +- 3 ms -> 175 ms +- 4 ms: 1.02x slower (+2%)
    • chameleon: 17.6 ms +- 0.3 ms -> 17.9 ms +- 0.2 ms: 1.02x slower (+2%)
    • telco: 11.3 ms +- 0.4 ms -> 11.5 ms +- 0.6 ms: 1.02x slower (+2%)
    • logging_simple: 19.0 us +- 0.3 us -> 19.2 us +- 0.3 us: 1.01x slower (+1%)
    • pickle_dict: 52.2 us +- 0.3 us -> 52.9 us +- 0.2 us: 1.01x slower (+1%)
    • pickle_list: 6.86 us +- 0.04 us -> 6.94 us +- 0.05 us: 1.01x slower (+1%)
    • pickle: 17.4 us +- 0.2 us -> 17.6 us +- 0.2 us: 1.01x slower (+1%)
    • logging_silent: 556 ns +- 13 ns -> 561 ns +- 11 ns: 1.01x slower (+1%)
    • sqlite_synth: 5.25 us +- 0.44 us -> 5.28 us +- 0.15 us: 1.01x slower (+1%)
    • pidigits: 248 ms +- 2 ms -> 249 ms +- 2 ms: 1.00x slower (+0%)
    • unpack_sequence: 73.5 ns +- 0.6 ns -> 73.8 ns +- 2.4 ns: 1.00x slower (+0%)

    Faster (27):

    • call_method_slots: 11.4 ms +- 0.1 ms -> 10.7 ms +- 0.1 ms: 1.07x faster (-6%)
    • call_method: 11.5 ms +- 0.1 ms -> 10.8 ms +- 0.2 ms: 1.06x faster (-6%)
    • call_simple: 10.7 ms +- 0.3 ms -> 10.1 ms +- 0.3 ms: 1.06x faster (-5%)
    • regex_effbot: 4.08 ms +- 0.05 ms -> 3.97 ms +- 0.04 ms: 1.03x faster (-3%)
    • regex_compile: 318 ms +- 11 ms -> 310 ms +- 10 ms: 1.03x faster (-3%)
    • html5lib: 154 ms +- 4 ms -> 150 ms +- 4 ms: 1.03x faster (-3%)
    • deltablue: 13.5 ms +- 0.5 ms -> 13.1 ms +- 0.4 ms: 1.03x faster (-3%)
    • call_method_unknown: 13.1 ms +- 0.1 ms -> 12.8 ms +- 0.1 ms: 1.02x faster (-2%)
    • richards: 131 ms +- 2 ms -> 128 ms +- 2 ms: 1.02x faster (-2%)
    • meteor_contest: 170 ms +- 3 ms -> 167 ms +- 4 ms: 1.02x faster (-2%)
    • sympy_integrate: 34.7 ms +- 0.7 ms -> 34.0 ms +- 0.2 ms: 1.02x faster (-2%)
    • spectral_norm: 210 ms +- 2 ms -> 206 ms +- 2 ms: 1.02x faster (-2%)
    • dulwich_log: 115 ms +- 1 ms -> 113 ms +- 2 ms: 1.01x faster (-1%)
    • genshi_text: 54.1 ms +- 0.8 ms -> 53.4 ms +- 0.5 ms: 1.01x faster (-1%)
    • 2to3: 511 ms +- 3 ms -> 505 ms +- 2 ms: 1.01x faster (-1%)
    • pathlib: 28.8 ms +- 0.6 ms -> 28.5 ms +- 0.8 ms: 1.01x faster (-1%)
    • xml_etree_parse: 199 ms +- 3 ms -> 198 ms +- 2 ms: 1.01x faster (-1%)
    • hexiom: 17.9 ms +- 0.1 ms -> 17.7 ms +- 0.1 ms: 1.01x faster (-1%)
    • mako: 35.1 ms +- 1.0 ms -> 34.9 ms +- 0.3 ms: 1.01x faster (-1%)
    • nbody: 194 ms +- 1 ms -> 193 ms +- 1 ms: 1.01x faster (-1%)
    • unpickle_pure_python: 634 us +- 15 us -> 630 us +- 10 us: 1.01x faster (-1%)
    • unpickle_list: 6.16 us +- 0.06 us -> 6.13 us +- 0.05 us: 1.01x faster (-1%)
    • genshi_xml: 113 ms +- 3 ms -> 112 ms +- 1 ms: 1.01x faster (-1%)
    • json_dumps: 19.1 ms +- 0.3 ms -> 19.1 ms +- 0.2 ms: 1.00x faster (-0%)
    • python_startup: 13.3 ms +- 0.1 ms -> 13.3 ms +- 0.1 ms: 1.00x faster (-0%)
    • python_startup_no_site: 8.24 ms +- 0.06 ms -> 8.22 ms +- 0.04 ms: 1.00x faster (-0%)
    • go: 421 ms +- 3 ms -> 421 ms +- 3 ms: 1.00x faster (-0%)

    Benchmark hidden because not significant (22): chaos, crypto_pyaes, django_template, fannkuch, float, json_loads, logging_format, nqueens, pickle_pure_python, raytrace, regex_dna, regex_v8, scimark_lu, scimark_sor, sqlalchemy_declarative, sqlalchemy_imperative, sympy_sum, tornado_http, unpickle, xml_etree_generate, xml_etree_iterparse, xml_etree_process

    @methane methane added the performance Performance or resource usage label Dec 22, 2016
    @methane
    Copy link
    Member Author

    methane commented Dec 22, 2016

    --enable-optimizations:
    $ ~/local/cpython/bin/pyperf compare_to -G default.json patched.json
    Slower (12):

    • pickle_dict: 42.0 us +- 0.2 us -> 42.9 us +- 0.2 us: 1.02x slower (+2%)
    • regex_dna: 228 ms +- 1 ms -> 233 ms +- 2 ms: 1.02x slower (+2%)
    • unpickle_list: 4.83 us +- 0.04 us -> 4.93 us +- 0.05 us: 1.02x slower (+2%)
    • nbody: 175 ms +- 1 ms -> 178 ms +- 1 ms: 1.02x slower (+2%)
    • unpack_sequence: 72.3 ns +- 0.4 ns -> 73.5 ns +- 4.8 ns: 1.02x slower (+2%)
    • scimark_sparse_mat_mult: 5.21 ms +- 0.09 ms -> 5.27 ms +- 0.04 ms: 1.01x slower (+1%)
    • dulwich_log: 97.2 ms +- 1.2 ms -> 98.2 ms +- 1.4 ms: 1.01x slower (+1%)
    • pickle_list: 5.69 us +- 0.04 us -> 5.73 us +- 0.07 us: 1.01x slower (+1%)
    • logging_format: 18.4 us +- 0.4 us -> 18.5 us +- 0.4 us: 1.01x slower (+1%)
    • meteor_contest: 148 ms +- 2 ms -> 149 ms +- 2 ms: 1.01x slower (+1%)
    • sympy_sum: 133 ms +- 4 ms -> 134 ms +- 5 ms: 1.00x slower (+0%)
    • python_startup: 12.3 ms +- 0.1 ms -> 12.3 ms +- 0.1 ms: 1.00x slower (+0%)

    Faster (42):

    • call_method_slots: 9.95 ms +- 0.33 ms -> 9.36 ms +- 0.38 ms: 1.06x faster (-6%)
    • call_method: 10.3 ms +- 0.4 ms -> 9.80 ms +- 0.53 ms: 1.05x faster (-5%)
    • pickle: 15.0 us +- 0.1 us -> 14.3 us +- 0.2 us: 1.05x faster (-5%)
    • call_method_unknown: 11.6 ms +- 0.3 ms -> 11.1 ms +- 0.4 ms: 1.04x faster (-4%)
    • spectral_norm: 195 ms +- 2 ms -> 188 ms +- 1 ms: 1.04x faster (-4%)
    • pathlib: 24.4 ms +- 0.3 ms -> 23.7 ms +- 0.3 ms: 1.03x faster (-3%)
    • json_dumps: 17.5 ms +- 0.5 ms -> 17.0 ms +- 0.3 ms: 1.03x faster (-3%)
    • regex_v8: 34.4 ms +- 0.2 ms -> 33.5 ms +- 0.4 ms: 1.03x faster (-3%)
    • call_simple: 9.26 ms +- 0.30 ms -> 9.01 ms +- 0.39 ms: 1.03x faster (-3%)
    • logging_silent: 456 ns +- 5 ns -> 444 ns +- 6 ns: 1.03x faster (-3%)
    • richards: 106 ms +- 2 ms -> 104 ms +- 2 ms: 1.02x faster (-2%)
    • django_template: 206 ms +- 8 ms -> 202 ms +- 7 ms: 1.02x faster (-2%)
    • sqlite_synth: 4.74 us +- 0.16 us -> 4.64 us +- 0.10 us: 1.02x faster (-2%)
    • scimark_fft: 467 ms +- 3 ms -> 457 ms +- 16 ms: 1.02x faster (-2%)
    • deltablue: 10.8 ms +- 0.2 ms -> 10.6 ms +- 0.2 ms: 1.02x faster (-2%)
    • regex_compile: 269 ms +- 5 ms -> 263 ms +- 2 ms: 1.02x faster (-2%)
    • genshi_xml: 98.5 ms +- 1.8 ms -> 96.7 ms +- 1.0 ms: 1.02x faster (-2%)
    • sympy_expand: 615 ms +- 9 ms -> 604 ms +- 11 ms: 1.02x faster (-2%)
    • tornado_http: 265 ms +- 4 ms -> 261 ms +- 4 ms: 1.02x faster (-2%)
    • scimark_sor: 298 ms +- 3 ms -> 294 ms +- 3 ms: 1.02x faster (-2%)
    • pickle_pure_python: 704 us +- 16 us -> 693 us +- 8 us: 1.02x faster (-1%)
    • crypto_pyaes: 154 ms +- 1 ms -> 152 ms +- 1 ms: 1.02x faster (-1%)
    • raytrace: 753 ms +- 8 ms -> 743 ms +- 8 ms: 1.01x faster (-1%)
    • nqueens: 140 ms +- 1 ms -> 138 ms +- 1 ms: 1.01x faster (-1%)
    • sympy_str: 277 ms +- 6 ms -> 274 ms +- 4 ms: 1.01x faster (-1%)
    • regex_effbot: 4.03 ms +- 0.12 ms -> 3.98 ms +- 0.06 ms: 1.01x faster (-1%)
    • html5lib: 130 ms +- 4 ms -> 129 ms +- 4 ms: 1.01x faster (-1%)
    • sqlalchemy_declarative: 237 ms +- 3 ms -> 234 ms +- 3 ms: 1.01x faster (-1%)
    • scimark_monte_carlo: 153 ms +- 2 ms -> 152 ms +- 1 ms: 1.01x faster (-1%)
    • hexiom: 14.8 ms +- 0.1 ms -> 14.7 ms +- 0.1 ms: 1.01x faster (-1%)
    • 2to3: 446 ms +- 2 ms -> 441 ms +- 2 ms: 1.01x faster (-1%)
    • mako: 29.0 ms +- 0.1 ms -> 28.8 ms +- 0.2 ms: 1.01x faster (-1%)
    • go: 346 ms +- 3 ms -> 343 ms +- 4 ms: 1.01x faster (-1%)
    • float: 184 ms +- 3 ms -> 183 ms +- 2 ms: 1.01x faster (-1%)
    • sqlalchemy_imperative: 49.2 ms +- 1.1 ms -> 48.7 ms +- 0.9 ms: 1.01x faster (-1%)
    • genshi_text: 46.5 ms +- 0.7 ms -> 46.1 ms +- 0.5 ms: 1.01x faster (-1%)
    • telco: 9.82 ms +- 0.38 ms -> 9.74 ms +- 0.17 ms: 1.01x faster (-1%)
    • chaos: 162 ms +- 1 ms -> 161 ms +- 1 ms: 1.01x faster (-1%)
    • sympy_integrate: 30.0 ms +- 0.2 ms -> 29.8 ms +- 0.2 ms: 1.01x faster (-1%)
    • fannkuch: 709 ms +- 2 ms -> 705 ms +- 1 ms: 1.01x faster (-1%)
    • scimark_lu: 322 ms +- 8 ms -> 320 ms +- 5 ms: 1.00x faster (-0%)
    • json_loads: 36.0 us +- 0.2 us -> 36.0 us +- 0.2 us: 1.00x faster (-0%)

    Benchmark hidden because not significant (10): chameleon, logging_simple, pidigits, python_startup_no_site, unpickle, unpickle_pure_python, xml_etree_generate, xml_etree_iterparse, xml_etree_parse, xml_etree_process

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Dec 24, 2016

    New changeset f5eb0c4f5d37 by INADA Naoki in branch 'default':
    Issue bpo-29049: Call _PyObject_GC_TRACK() lazily when calling Python function.
    https://hg.python.org/cpython/rev/f5eb0c4f5d37

    @methane methane closed this as completed Dec 24, 2016
    @methane
    Copy link
    Member Author

    methane commented Dec 24, 2016

    (Rietveld shows 500 error when replying.)

    Minor style note: Use Py_REFCNT here to simplify?

    Fixed when committing.

    @serhiy-storchaka
    Copy link
    Member

    Please wrap private functions in #ifndef Py_LIMITED_API/#endif.

    @methane
    Copy link
    Member Author

    methane commented Dec 25, 2016

    Please wrap private functions in #ifndef Py_LIMITED_API/#endif.

    No problem. Whole Include/frameobject.h is wrapped by Py_LIMITED_API.

    @serhiy-storchaka
    Copy link
    Member

    Ah, sorry. Then it LGTM.

    Do you have a microbenchmark that exposes a maximal effect of this change?

    @serhiy-storchaka
    Copy link
    Member

    Perhaps this change caused reference leaks:

    test_exceptions leaked [1227, 1227, 1227] references, sum=3681
    test_exceptions leaked [342, 344, 344] memory blocks, sum=1030
    test_capi leaked [6043, 6043, 6043] references, sum=18129
    test_capi leaked [1715, 1717, 1717] memory blocks, sum=5149
    test_cgi leaked [68, 68, 68] references, sum=204
    test_cgi leaked [34, 34, 35] memory blocks, sum=103
    test_collections leaked [0, -7, 8] memory blocks, sum=1
    test_contextlib leaked [2162, 2162, 2162] references, sum=6486
    test_contextlib leaked [635, 637, 637] memory blocks, sum=1909
    test_functools leaked [0, 3, 1] memory blocks, sum=4
    test_importlib leaked [3068, 3068, 3068] references, sum=9204
    test_importlib leaked [936, 938, 938] memory blocks, sum=2812
    test_inspect leaked [34, 34, 34] references, sum=102
    test_inspect leaked [10, 11, 10] memory blocks, sum=31
    test_os leaked [182, 182, 182] references, sum=546
    test_os leaked [112, 112, 112] memory blocks, sum=336
    test_ssl leaked [3403, 3406, 3406] references, sum=10215
    test_ssl leaked [977, 980, 980] memory blocks, sum=2937
    test_threading leaked [12086, 12086, 12086] references, sum=36258
    test_threading leaked [3430, 3432, 3432] memory blocks, sum=10294
    test_timeout leaked [701, 701, 701] references, sum=2103
    test_timeout leaked [203, 204, 204] memory blocks, sum=611
    test_xml_etree leaked [3937, 3937, 3937] references, sum=11811
    test_xml_etree leaked [1517, 1520, 1521] memory blocks, sum=4558
    test_xml_etree_c leaked [354, 354, 354] references, sum=1062
    test_xml_etree_c leaked [216, 217, 217] memory blocks, sum=650
    test_yield_from leaked [33, 33, 33] references, sum=99
    test_yield_from leaked [8, 8, 8] memory blocks, sum=24

    @zhangyangyu
    Copy link
    Member

    On my PC, this patch also introduces two tests failure:

    FAIL: test_log_destroyed_pending_task (test.test_asyncio.test_tasks.CTask_CFuture_SubclassTests)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/home/angwer/cpython/Lib/test/test_asyncio/test_tasks.py", line 1823, in test_log_destroyed_pending_task
        self.assertEqual(Task.all_tasks(loop=self.loop), set())
    AssertionError: Items in the first set but not the second:
    <Task pending coro=<BaseTaskTests.test_log_destroyed_pending_task.<locals>.kill_me() running at /home/angwer/cpython/Lib/test/test_asyncio/test_tasks.py:1796> wait_for=<Future pending cb=[<TaskWakeupMethWrapper object at 0x7f649f135f18>()] created at /home/angwer/cpython/Lib/test/test_asyncio/test_tasks.py:2001> created at /home/angwer/cpython/Lib/test/test_asyncio/test_tasks.py:83>

    ======================================================================
    FAIL: test_log_destroyed_pending_task (test.test_asyncio.test_tasks.CTask_CFuture_Tests)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/home/angwer/cpython/Lib/test/test_asyncio/test_tasks.py", line 1823, in test_log_destroyed_pending_task
        self.assertEqual(Task.all_tasks(loop=self.loop), set())
    AssertionError: Items in the first set but not the second:
    <Task pending coro=<BaseTaskTests.test_log_destroyed_pending_task.<locals>.kill_me() running at /home/angwer/cpython/Lib/test/test_asyncio/test_tasks.py:1796> wait_for=<Future pending cb=[<TaskWakeupMethWrapper object at 0x7f649f135498>()] created at /home/angwer/cpython/Lib/test/test_asyncio/test_tasks.py:86> created at /home/angwer/cpython/Lib/asyncio/base_events.py:287>

    ======================================================================
    FAIL: test_log_destroyed_pending_task (test.test_asyncio.test_tasks.CTask_PyFuture_Tests)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/home/angwer/cpython/Lib/test/test_asyncio/test_tasks.py", line 1823, in test_log_destroyed_pending_task
        self.assertEqual(Task.all_tasks(loop=self.loop), set())
    AssertionError: Items in the first set but not the second:
    <Task pending coro=<BaseTaskTests.test_log_destroyed_pending_task.<locals>.kill_me() running at /home/angwer/cpython/Lib/test/test_asyncio/test_tasks.py:1796> wait_for=<Future pending cb=[<TaskWakeupMethWrapper object at 0x7f649f135d98>()] created at /home/angwer/cpython/Lib/test/test_asyncio/test_tasks.py:86> created at /home/angwer/cpython/Lib/asyncio/base_events.py:287>

    ======================================================================
    FAIL: test_log_destroyed_pending_task (test.test_asyncio.test_tasks.PyTask_CFuture_Tests)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/home/angwer/cpython/Lib/test/test_asyncio/test_tasks.py", line 1823, in test_log_destroyed_pending_task
        self.assertEqual(Task.all_tasks(loop=self.loop), set())
    AssertionError: Items in the first set but not the second:
    <Task pending coro=<BaseTaskTests.test_log_destroyed_pending_task.<locals>.kill_me() running at /home/angwer/cpython/Lib/test/test_asyncio/test_tasks.py:1796> wait_for=<Future pending cb=[Task._wakeup()] created at /home/angwer/cpython/Lib/test/test_asyncio/test_tasks.py:86> created at /home/angwer/cpython/Lib/asyncio/base_events.py:287>

    ======================================================================
    FAIL: test_log_destroyed_pending_task (test.test_asyncio.test_tasks.PyTask_PyFuture_SubclassTests)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/home/angwer/cpython/Lib/test/test_asyncio/test_tasks.py", line 1823, in test_log_destroyed_pending_task
        self.assertEqual(Task.all_tasks(loop=self.loop), set())
    AssertionError: Items in the first set but not the second:
    <Task pending coro=<BaseTaskTests.test_log_destroyed_pending_task.<locals>.kill_me() running at /home/angwer/cpython/Lib/test/test_asyncio/test_tasks.py:1796> wait_for=<Future pending cb=[add_subclass_tests.<locals>.Task._wakeup()] created at /home/angwer/cpython/Lib/test/test_asyncio/test_tasks.py:2001> created at /home/angwer/cpython/Lib/test/test_asyncio/test_tasks.py:83>

    ======================================================================
    FAIL: test_log_destroyed_pending_task (test.test_asyncio.test_tasks.PyTask_PyFuture_Tests)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/home/angwer/cpython/Lib/test/test_asyncio/test_tasks.py", line 1823, in test_log_destroyed_pending_task
        self.assertEqual(Task.all_tasks(loop=self.loop), set())
    AssertionError: Items in the first set but not the second:
    <Task pending coro=<BaseTaskTests.test_log_destroyed_pending_task.<locals>.kill_me() running at /home/angwer/cpython/Lib/test/test_asyncio/test_tasks.py:1796> wait_for=<Future pending cb=[Task._wakeup()] created at /home/angwer/cpython/Lib/test/test_asyncio/test_tasks.py:86> created at /home/angwer/cpython/Lib/asyncio/base_events.py:287>

    ======================================================================
    FAIL: test_refcycle (test.test_generators.FinalizationTest)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/home/angwer/cpython/Lib/test/test_generators.py", line 54, in test_refcycle
        self.assertTrue(finalized)
    AssertionError: False is not true

    test test_generators failed
    test_generators failed

    2 tests failed:
    test_asyncio test_generators

    Total duration: 17 sec
    Tests result: FAILURE

    @zhangyangyu
    Copy link
    Member

    I don't have time study the patches. But 29049-fix-generator-refleak.patch fixes the two test failures on my PC. :-)

    @serhiy-storchaka
    Copy link
    Member

    LGTM.

    It seems to me, that the part with Py_DECREF should be pushed to older branches.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Dec 26, 2016

    New changeset d6913acb31e3 by INADA Naoki in branch 'default':
    Issue bpo-29049: Fix refleak introduced by f5eb0c4f5d37.
    https://hg.python.org/cpython/rev/d6913acb31e3

    @methane
    Copy link
    Member Author

    methane commented Dec 26, 2016

    It seems to me, that the part with Py_DECREF should be pushed to older branches.

    PyCoro_New() and PyGen_NewWithQualName() steals f reference, and DECREF when error.
    So Py_DECREF is not needed when (gen == NULL). My patch was wrong.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Dec 26, 2016

    New changeset 5f3ac68f34f2 by INADA Naoki in branch 'default':
    Issue bpo-29049: Remove unnecessary Py_DECREF
    https://hg.python.org/cpython/rev/5f3ac68f34f2

    @methane methane closed this as completed Dec 26, 2016
    @vstinner
    Copy link
    Member

    FYI this optimization triggered a bug in the tp_clear slot of ElementTree parsers, bug which was hidden for years in Python 3: bpo-31499.

    @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

    4 participants