GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-47009: Let PRECALL_NO_KW_LIST_APPEND do its own POP_TOP by sweeneyde · Pull Request #32239 · python/cpython

Conversation

@sweeneyde

@sweeneyde sweeneyde commented Apr 1, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Most code won't do y = L.append(x) or whatnot, so PRECALL_NO_KW_LIST_APPEND is almost always followed by POP_TOP. We can verify at specialization time.

This saves a Py_INCREF(Py_None), a SET_TOP(Py_None), and POP_TOP's Py_DECREF(POP()); DISPATCH();.

Some microbenchmarks:

from pyperf import Runner, perf_counter

def bench_append(loops, length):
    src = list(map(float, range(length)))
    arr = []
    t0 = perf_counter()

    for i in range(loops):
        arr.clear()
        for x in src:
            arr.append(x)

    return perf_counter() - t0

def bench_append_less_gc(loops, length):
    src = list(map(float, range(length)))
    out = [None] * loops
    t0 = perf_counter()

    for i in range(loops):
        arr = []
        for x in src:
            arr.append(x)
        out[i] = arr

    return perf_counter() - t0

runner = Runner()
for n in [100, 1_000, 10_000, 100_000]:
    runner.bench_time_func(f"append {n}", bench_append, n, inner_loops=n)
    runner.bench_time_func(f"append-less-gc {n}", bench_append_less_gc, n, inner_loops=n)

From GCC, --enable-optimizations, --with-lto:

- append 100000: 14.9 ns +- 0.3 ns -> 13.3 ns +- 0.4 ns: 1.12x faster
- append 10000: 15.1 ns +- 0.3 ns -> 13.6 ns +- 0.5 ns: 1.11x faster
- append-less-gc 100000: 16.4 ns +- 0.5 ns -> 14.9 ns +- 0.4 ns: 1.10x faster
- append 1000: 15.6 ns +- 0.3 ns -> 14.2 ns +- 0.3 ns: 1.09x faster
- append 100: 18.9 ns +- 0.6 ns -> 17.3 ns +- 0.6 ns: 1.09x faster
- append-less-gc 100: 27.4 ns +- 1.1 ns -> 25.2 ns +- 1.2 ns: 1.09x faster
- append-less-gc 10000: 19.2 ns +- 0.3 ns -> 17.8 ns +- 0.2 ns: 1.08x faster
- append-less-gc 1000: 22.0 ns +- 0.6 ns -> 20.8 ns +- 0.3 ns: 1.06x faster

Geometric mean: 1.09x faster

https://bugs.python.org/issue47009

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@sweeneyde
Let PRECALL_NO_KW_LIST_APPEND do its own POP_TOP
344ee45
@sweeneyde
sweeneyde requested a review from markshannon as a code owner April 1, 2022 18:37
@bedevere-bot bedevere-bot added the awaiting core review label Apr 1, 2022
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Apr 1, 2022
@sweeneyde sweeneyde added the skip news label Apr 1, 2022
@sweeneyde
Fix a copy/paste error
e0adb6a
@sweeneyde
sweeneyde force-pushed the listappend_pop branch from 43782cc to e0adb6a Compare April 1, 2022 20:51
@sweeneyde
Merge branch 'main' into listappend_pop
3361295
@sweeneyde
sweeneyde requested a review from brandtbucher April 5, 2022 07:19
@markshannon

markshannon commented Apr 5, 2022

Copy link
Copy Markdown
Member

Looks good. I'm a bit wary of specialized superinstructions, but this seems solid.
I can imagine cases where list.append() wouldn't be followed by a POP_TOP, but they are contrived and highly unlikely.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@markshannon
markshannon merged commit 6c6e040 into python:main Apr 5, 2022
@bedevere-bot bedevere-bot removed the awaiting core review label Apr 5, 2022
@tiran

tiran commented Apr 5, 2022

Copy link
Copy Markdown
Member

The assert is failing on s390x Fedora buildbot https://buildbot.python.org/all/#/builders/232/builds/524

_bootstrap_python: Python/ceval.c:5045: _PyEval_EvalFrameDefault: Assertion `next_instr[-1] == POP_TOP' failed.
make: *** [Makefile:1204: Python/frozen_modules/io.h] Aborted (core dumped)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@markshannon

markshannon commented Apr 5, 2022

Copy link
Copy Markdown
Member

Strange. The bytecode is exactly the same on all platforms.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sweeneyde
sweeneyde deleted the listappend_pop branch April 5, 2022 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner
@brandtbucher brandtbucher Awaiting requested review from brandtbucher

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@sweeneyde @markshannon @tiran @the-knights-who-say-ni @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.