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

The bytecode for f-string formatting is inefficient. #77273

Closed
markshannon opened this issue Mar 17, 2018 · 9 comments
Closed

The bytecode for f-string formatting is inefficient. #77273

markshannon opened this issue Mar 17, 2018 · 9 comments
Labels
3.12 bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage

Comments

@markshannon
Copy link
Member

BPO 33092
Nosy @pitrou, @ericvsmith, @markshannon, @serhiy-storchaka
PRs
  • gh-77273: Better bytecodes for f-strings #6132
  • 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 = None
    created_at = <Date 2018-03-17.12:33:10.371>
    labels = ['interpreter-core', '3.8', 'performance']
    title = 'The bytecode for f-string formatting is inefficient.'
    updated_at = <Date 2021-08-26.14:31:11.408>
    user = 'https://github.com/markshannon'

    bugs.python.org fields:

    activity = <Date 2021-08-26.14:31:11.408>
    actor = 'Mark.Shannon'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Interpreter Core']
    creation = <Date 2018-03-17.12:33:10.371>
    creator = 'Mark.Shannon'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 33092
    keywords = ['patch']
    message_count = 7.0
    messages = ['314000', '314001', '314004', '314312', '314317', '322379', '400348']
    nosy_count = 4.0
    nosy_names = ['pitrou', 'eric.smith', 'Mark.Shannon', 'serhiy.storchaka']
    pr_nums = ['6132']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'performance'
    url = 'https://bugs.python.org/issue33092'
    versions = ['Python 3.8']

    @markshannon
    Copy link
    Member Author

    f-string expressions can be formatted in four ways:
    with or without a conversion
    and
    with or without a format specifier

    Rather than have one bytecode that parses the opcode argument at runtime it would be more efficient and produce a cleaner interpreter for the compiler to produce one or two bytecode as required.
    The bytecodes should be:
    CONVERT_VALUE convert_fn
    FORMAT_SIMPLE
    FORMAT_WITH_SPEC

    For simple format expressions with no conversion or format specifier,
    which make up about 3/4 of all format expressions in the standard library, just the bytecode FORMAT_SIMPLE need be executed.

    @markshannon markshannon added 3.8 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage labels Mar 17, 2018
    @ericvsmith
    Copy link
    Member

    Would this change really have a performance impact? Not saying it wouldn't, but if we're going to add bytecodes we should know the answer in advance.

    @markshannon
    Copy link
    Member Author

    Even if doesn't speed things up by a significant amount, I would suggest that a simper interpreter with smaller, simpler bytecodes is a worthy goal in itself.

    @pitrou
    Copy link
    Member

    pitrou commented Mar 23, 2018

    I would suggest that a simper interpreter with smaller, simpler bytecodes is a worthy goal in itself.

    +1 from me. Though I'm curious about performance changes as well :-)

    @serhiy-storchaka
    Copy link
    Member

    I wouldn't say this more efficient. Instead one instruction you would need to execute two instructions.

    If I implemented f-string formatting I would add four simple opcodes for str(), repr(), ascii() and format(). But Eric merged them all in the single opcode with complex argument. While this looks more complicated and less extensible, it is more efficient.

    @taleinat
    Copy link
    Contributor

    Mark, since you have a working version of this, perhaps you can supply some performance benchmark results to help in making a decision?

    @markshannon
    Copy link
    Member Author

    No significant change in performance https://gist.github.com/markshannon/34a780d65e69b5a573a83f3fdb0139aa

    I think this merely indicates that there are little to no f-strings in the pyperformance benchmark suite.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @iritkatriel iritkatriel added 3.12 bugs and security fixes and removed 3.8 (EOL) end of life labels Sep 1, 2022
    @iritkatriel
    Copy link
    Member

    @markshannon are you still interested in this? should we repeat the measurements with benchmarks that have more f-strings?

    carljm added a commit to carljm/cpython that referenced this issue Jun 15, 2023
    * main: (57 commits)
      pythongh-105831: Fix NEWS blurb from pythongh-105828 (python#105833)
      pythongh-105820: Fix tok_mode expression buffer in file & readline tokenizer (python#105828)
      pythongh-105751, test_ctypes: Remove disabled tests (python#105826)
      pythongh-105821: Use a raw f-string in test_httpservers.py (python#105822)
      pythongh-105751: Remove platform usage in test_ctypes (python#105819)
      pythongh-105751: Reenable disable test_ctypes tests (python#105818)
      pythongh-105751: Remove dead code in test_ctypes (python#105817)
      More reorganisation of the typing docs (python#105787)
      Improve docs for `typing.dataclass_transform` (python#105792)
      pythonGH-89812: Churn `pathlib.Path` test methods (python#105807)
      pythongh-105800: Issue SyntaxWarning in f-strings for invalid escape sequences (python#105801)
      pythongh-105751: Cleanup test_ctypes imports (python#105803)
      pythongh-105481: add HAS_JUMP flag to opcode metadata (python#105791)
      pythongh-105751: test_ctypes avoids the operator module (pythonGH-105797)
      pythongh-105751: test_ctypes: Remove @need_symbol decorator (pythonGH-105798)
      pythongh-104909: Implement conditional stack effects for macros (python#105748)
      pythongh-75905: Remove test_xmlrpc_net: skipped since 2017 (python#105796)
      pythongh-105481: Fix types and a bug for pseudos (python#105788)
      Update DSL docs for cases generator (python#105753)
      pythonGH-77273: Better bytecodes for f-strings (pythonGH-6132)
      ...
    @hauntsaninja
    Copy link
    Contributor

    Looks like this was implemented in #6132, thanks all!

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.12 bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage
    Projects
    None yet
    Development

    No branches or pull requests

    7 participants