Skip to content

Improve performance of find_max_char #122901

Open
Open
Improve performance of find_max_char#122901
Labels
performancePerformance or resource usagetype-featureA feature request or enhancement

Description

@ruema
ruema
opened on Aug 11, 2024

Feature or enhancement

Proposal:

find_max_char is called each time a string is created.
By reducing the amount of tests, the performance can be improved considerably.

python -m pyperf timeit -s "b=('a' *1000+ '\u019f'*2)" "b[:-1]"

+-----------+--------+----------------------+
| Benchmark | ref    | patch                |
+===========+========+======================+
| timeit    | 442 ns | 388 ns: 1.14x faster |
+-----------+--------+----------------------+

Has this already been discussed elsewhere?

This is a minor feature, which does not need previous discussion elsewhere

Links to previous discussion of this feature:

No response

Linked PRs

Activity

  1. ruema
    added
    type-featureA feature request or enhancement
    on Aug 11, 2024
  2. aisk
    added
    performancePerformance or resource usage
    on Aug 11, 2024

  3. terryjreedy commented on Aug 11, 2024

    @terryjreedy
    terryjreedy
    on Aug 11, 2024
    Member

    @vstinner Should you or someone else look at this? Should OP say more about idea of patch? Is the above the proper speed test?

  4. picnixz commented on Aug 13, 2024

    @picnixz
    picnixz
    on Aug 13, 2024
    Member

    I would also look at the arguments in #120212.

    Personally, I would include in the benchmarks:

    • ASCII data only
    • lots of non-ASCII data
    • a bit of non-ASCII data
    • short, medium and very long strings

    By the way, is your build a release or a PGO (maybe with LTO) build?

  5. vstinner commented on Aug 27, 2024

    @vstinner
    vstinner
    on Aug 27, 2024
    Member

    @ruema: Would you mind to run more benchmarks? See previous @picnixz comment about that.

  6. rhpvorderman commented on Sep 10, 2024

    @rhpvorderman
    rhpvorderman
    on Sep 10, 2024
    Contributor

    This is basically the same as #120212 but with larger chunks. Given that this approach unifies the find_max_char functions for all
    char sizes, from a maintainability standpoint it is better than #120212.

    I would add benchmarks for line by line reading. for line in file_descriptor is a very common pattern.

  7. vstinner commented on Sep 18, 2024

    @vstinner
    vstinner
    on Sep 18, 2024
    Member

    Would it be possible to have a few more benchmarks on this change to have an idea if it's worth it or not?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    performancePerformance or resource usagetype-featureA feature request or enhancement

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions