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

Fix deduplication of tuples constaining IdentifierStringNode #4353

Merged
merged 2 commits into from
Aug 31, 2021

Conversation

da-woods
Copy link
Contributor

I'm hoping to speed up the complication of some of the larger
Cython internal modules (but it's worthwhile even if that fails
I think!). ExprNodes.py was creating hundreds
of tuples containing ("self", "env") for example, because it
wasn't able to match and deduplicate them.

I'm hoping to speed up the complication of some of the larger
Cython internal modules (but it's worthwhile even if that fails
I think!). ExprNodes.py was creating hundreds
of tuples containing ("self", "env") for example, because it
wasn't able to match and deduplicate them.
@da-woods
Copy link
Contributor Author

It doesn't appear to change the compilation time noticeably (so fails in my initial aim) but I think it's still worthwhile to do.

# 1. it doesn't usually have unicode_value
# 2. it's often created later in the compilation process after ConstantFolding
# but should be cacheable
else (node.type, node.value, node.unicode_value, "IdentifierStringNode") if isinstance(node, IdentifierStringNode)
Copy link
Contributor

Choose a reason for hiding this comment

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

This whole if-chain is basically just special-casing expression nodes. Having to add yet another case reminds me that we should really let the nodes build the key themselves. Probably with one or two helper functions for the actual tuple building.

I'll merge it as it is, but we should keep the cleanup in mind.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, we're getting closer to Yandere Dev's inimitable skill ^_^

@scoder scoder added this to the 3.0 milestone Aug 31, 2021
@scoder scoder merged commit bad93a3 into cython:master Aug 31, 2021
@scoder
Copy link
Contributor

scoder commented Aug 31, 2021

Nice one. Thanks!

@da-woods da-woods deleted the dedup-string branch August 31, 2021 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants