-
Notifications
You must be signed in to change notification settings - Fork 22
use pyo3::intern!
#66
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
Conversation
905c844 to
8367255
Compare
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.
Food for thought: would it be a good idea to dedupe interns
pyo3::intern!(py, "run_until_complete")appears twice? would dedupling help?
Deduping is potentially useful if it's easy to do; it will reduce the amount of static storage used (by the interior of the macro). I think that the same Python string would be shared between duplicate uses of intern!, though.
|
@davidhewitt I (also) think it would work? but I certainly dont know better than you! Would inlining possibly cause problems? I have been sharing (unpaid) interns (here and also-here) and it seems to work good? EDIT: to clarify, I have been meaning to ask you or make a pyo3 discussion to figure out of intern-deduping actually does anything. raw links |
8367255 to
f2df6b2
Compare
|
@davidhewitt updated the branch... let me know if I should dedupe |
|
@davidhewitt any thoughts on this? is it good to go? |
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.
LGTM, thanks! Sorry for the delay.
|
No prob. |
Added
pyo3::intern!()where applicable.I imagine this would be beneficial in that some of these
call_method/getattr/etc places get called often.Food for thought: would it be a good idea to dedupe interns
pyo3::intern!(py, "run_until_complete")appears twice? would dedupling help?PS: obviously these are unpaid interns