Skip to content

PyStrInterned for PyAttributes/Constants #3700

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
May 26, 2022
Merged

PyStrInterned for PyAttributes/Constants #3700

merged 9 commits into from
May 26, 2022

Conversation

youknowone
Copy link
Member

@youknowone youknowone commented May 12, 2022 •
edited
Loading

2.5% performance gain for pystone

  • first bit of RefCount is now used to mark leaked objects
  • identifier!() macro to refer static identifier
  • PyStrInterned impl DictKey

@coolreader18
Copy link
Member

Might make sense to call these identifiers, like the _Py_IDENTIFIER macro.

@youknowone
Copy link
Member Author

Do you mean changing interned macro to PyIdentifier? This is making sense.
Or something like _Py_IDENTIFIER to define them? This is not working because we don't have global variables. (Any clever idea?)

@youknowone youknowone marked this pull request as ready for review May 12, 2022 10:47
@youknowone
Copy link
Member Author

@coolreader18 I am sorry, but I think I wasn't getting what you meant correctly. What do you mean by "to call these identifiers"?

@youknowone youknowone force-pushed the attr-intern branch 2 times, most recently from 0842726 to c7bca44 Compare May 12, 2022 13:10
@youknowone youknowone mentioned this pull request May 12, 2022
Open
6 tasks
@coolreader18
Copy link
Member

Oh, just in the names, not like change to be the same functionality as the _Py_IDENTIFIER

@youknowone
Copy link
Member Author

not easy to solve static type problems

@youknowone
Copy link
Member Author

@coolreader18 do you have a good idea?

@youknowone youknowone mentioned this pull request May 15, 2022
Merged
@youknowone youknowone marked this pull request as draft May 16, 2022 20:05
@youknowone
Copy link
Member Author

@coolreader18 does identifier! look good? py_identifier!?

@youknowone youknowone marked this pull request as ready for review May 16, 2022 21:12
@youknowone youknowone force-pushed the attr-intern branch 3 times, most recently from 947fba6 to 2e6a6ed Compare May 16, 2022 22:00
@youknowone youknowone changed the title PyStrInterned PyStrInterned for PyAttributes May 16, 2022
@youknowone youknowone changed the title PyStrInterned for PyAttributes PyStrInterned for PyAttributes/Constants May 16, 2022
@youknowone
Copy link
Member Author

@coolreader18 done

@youknowone youknowone force-pushed the attr-intern branch 5 times, most recently from 72d87e4 to 6afc01c Compare May 17, 2022 10:56
@youknowone youknowone mentioned this pull request May 18, 2022
Merged
@youknowone youknowone force-pushed the attr-intern branch 3 times, most recently from 1df4369 to ff7a66f Compare May 18, 2022 22:48
@youknowone youknowone requested a review from coolreader18 May 24, 2022 02:05
@youknowone youknowone merged commit c44843f into RustPython:main May 26, 2022
@youknowone youknowone deleted the attr-intern branch May 26, 2022 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@coolreader18 coolreader18 Awaiting requested review from coolreader18

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@youknowone @coolreader18