-
Notifications
You must be signed in to change notification settings - Fork 778
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
Add PyString::intern to enable access to Python's built-in string interning. #2268
Conversation
68c0e5f
to
b04a157
Compare
I'd at least add a version that takes |
b04a157
to
d539d24
Compare
Do you mean a function taking
While that is true for the current implementation, I think this is actually a performance bug as that implementation should probably use Or put differently, if this is guaranteed to allocate a temporary |
Either that, or one creating the intermediary CString by itself. It could even do both, by trying first to create a |
Sorry I wasn't thinking properly about the "benchmark dict access with strings" use case. In general, having an API for interning is still useful for other cases, so I'd like this to go forward independently. |
Added a separate commit providing a more convenient API using |
Thanks! BTW, I can never figure out what people mean by the "eyes" reaction, what's your take on that? |
The meaning I wanted to convey was "I am looking into that", so when I am currently thinking about/working on something but do not have any positive or negative results to share yet. |
@mejrs @birkenfeld I rebooted this into a wrapper that is purely targeted at memory efficiency as |
25a047d
to
3965d1a
Compare
LGTM. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
I think the API is somewhat unfortunate but I am not sure how to get around the requirement for a null-terminated string without the cost of the allocation that this API is supposed to avoid in the first place. At least not without a
PyUnicode_InternFromStringAndSize
.