gh-124153: Fix unstable optimization of PyType_GetBaseByToken() and friends#124488
Merged
encukou merged 14 commits intopython:mainfrom Oct 10, 2024
neonene:opttoken
Merged
gh-124153: Fix unstable optimization of PyType_GetBaseByToken() and friends#124488encukou merged 14 commits intopython:mainfrom neonene:opttoken
PyType_GetBaseByToken() and friends#124488encukou merged 14 commits intopython:mainfrom
neonene:opttoken
Conversation
6 tasks
encukou
reviewed
Sep 25, 2024
Contributor
Author
|
One of suspects that exhausts MSVC: static PyTypeObject *
-get_base_by_token_recursive(PyTypeObject *type, void *token)
+get_base_by_token_recursive(PyObject *bases, void *token) |
PyTuple_GET_SIZE() calls PyTuple_Check()
Contributor
Author
|
@encukou Please review again. This should not be bad for performance on average. |
Including the revert: 2e7bb29
Contributor
Author
|
Bad example: PyTypeObject *base = NULL;
if (((PyHeapTypeObject*)type)->ht_token == token) {
found:
if (result != NULL) {
*result = (PyTypeObject *)Py_NewRef(base ? base : type);
}
return 1;
}Are you interested in removing the checker function like the current PR? If not, I'll give up. |
not to impact PyType_GetSlot() and others.
encukou
approved these changes
Oct 10, 2024
Member
encukou
left a comment
encukou
left a comment
There was a problem hiding this comment.
This looks good! Thank you, and apologies for the delay.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
An alternative PR to #124323.
This helps MSVC to optimize
PyType_GetBaseByToken()andPyType_GetModuleByDef()on PGO builds without current workaround.cc @encukou
PyType_GetBaseByTokenfunction withPy_tp_tokenslot #124153