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

Refactoring: make the type of fullname str instead of Bogus[str] #14435

Merged
merged 2 commits into from
Jan 13, 2023

Conversation

JukkaL
Copy link
Collaborator

@JukkaL JukkaL commented Jan 12, 2023

The type Bogus[X] is treated as Any when the code is compiled with mypyc, while it's equivalent to X when only type checking. They are sometimes used when X is not actually the real type of a value, but changing it to the correct type would be complicated.

Bogus[str] types are pretty awkward, since we are lying to the type checker and mypyc only sees Any types.

An empty fullname is now represented by "" instead of None, so we no longer need a Bogus[str] type.

This might break some plugins, so we should document this in release notes and the relevant issue that tracks plugin incompatibilities.

(Various small optimizations, including this, together netted a 6% performance improvement in self check.)

The type Bogus[X] is treated as Any when the code is compiled with mypyc,
while it's equivalent to X when only type checking. They are sometimes
used when X is not actually the real type of a value, but changing it to
the correct type would be complicated.

Bogus[str] types are pretty awkward, since we are lying to the type
checker and mypyc only sees Any types.

An empty fullname is now represented by "" instead of None, so we
no longer need a Bogus[str] type.

(Various small optimizations, including this, together netted a 6%
performance improvement in self check.)
@github-actions

This comment has been minimized.

Copy link
Member

@ilevkivskyi ilevkivskyi 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!

@msullivan
Copy link
Collaborator

6%? wow

@JukkaL
Copy link
Collaborator Author

JukkaL commented Jan 13, 2023

6%? wow

These are only the gains from small micro-optimizations where the impact of individual optimizations was hard to measure in isolation, so I measured the impact of a bunch of them together. Other optimizations yielded better results. Currently we are at 40% overall improvement in self check performance since the lowest perf level recorded here: https://github.com/mypyc/mypyc-benchmark-results/blob/master/reports/benchmarks/mypy_self_check.md

@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@JukkaL JukkaL merged commit 28e5436 into master Jan 13, 2023
@JukkaL JukkaL deleted the faster-bogus-str branch January 13, 2023 11:39
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.

3 participants