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

[mypyc] Add a special case for str_rprimitive in builder.builtin_len #10710

Merged
merged 14 commits into from
Jul 1, 2021

Conversation

97littleleaf11
Copy link
Collaborator

Description

Closes mypyc/mypyc#835

  • Add a branch for str_rpimitive in builtin_len
  • Reduce redundant code
  • Faster list/tuple built from str

Test Plan

  • Add run tests for faster list/tuple built from str
  • Modify one irbuild test. The original one is redundant.

source_str = 'abbc'
b = tuple('s:' + x for x in source_str)
assert b == ('s:a', 's:b', 's:b', 's:c')

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a mistake last week that a cannot be assigned to Tuple[str...].

@@ -350,14 +350,13 @@ L4:
return 1


[case testTupleBuiltFromList2]
[case testTupleBuiltFromStr]
def f2(val: str) -> str:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is another testTupleBuiltFromList thus it's redundant.

@JukkaL
Copy link
Collaborator

JukkaL commented Jun 28, 2021

Can you double check the perf impact by running some quick microbenchmark?

@97littleleaf11
Copy link
Collaborator Author

This branch:

interpreted: 0.000373s (avg of 2601 iterations; stdev 1.6%)
compiled:    0.000180s (avg of 2601 iterations; stdev 1.3%)

compiled is 2.070x faster

Master branch:

interpreted: 0.000357s (avg of 2675 iterations; stdev 4.6%)
compiled:    0.000233s (avg of 2675 iterations; stdev 2.2%)

compiled is 1.529x faster

Tested on:

def list_from_str() -> None:
    s1 = "aaaaa"
    n = 0
    for i in range(1000):
        list = [x for x in s1]
        n += len(list)
    assert n == 5000, n

@97littleleaf11
Copy link
Collaborator Author

Adding a wrapper function has little impact on performance, since the speed up is based on the optimized list comprehension:

interpreted: 0.000327s (avg of 2838 iterations; stdev 1.4%)
compiled:    0.000157s (avg of 2838 iterations; stdev 1.2%)

compiled is 2.084x faster

mypyc/lib-rt/str_ops.c Outdated Show resolved Hide resolved
Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thanks for the updates.

@JukkaL JukkaL merged commit 49bb90a into python:master Jul 1, 2021
@97littleleaf11 97littleleaf11 deleted the builtin-len-str branch July 6, 2021 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add specialization for strings in builtin_len
3 participants