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

gh-124153: Clean up workarounds for PyType_GetBaseByToken() performance #124323

Closed
wants to merge 7 commits into from
Closed

gh-124153: Clean up workarounds for PyType_GetBaseByToken() performance #124323

wants to merge 7 commits into from

Conversation

neonene
Copy link
Contributor

@neonene neonene commented Sep 23, 2024

This moves down PyType_GetBaseByToken()'s sub functions in typeobject.c.

Regarding MSVC, most of the PGO performance depends on the initial layout of *.obj file compiled at the collection phase (PGInsturment). Hopefully, PyType_GetBaseByToken() could have a stable performance, once we find a good formation of the family, as the performance defference between 3.9 and 3.10.X, which has come from the ceval.c layout, is still constant in a good PGO health.

Without this, any functions in typeobject.c can be slower if they have a similar branch:

if (!_PyType_HasFeature(foo, Py_TPFLAGS_HEAPTYPE)) {
    ...
} 
...  // no else

Currently, PyType_GetSlot(), PyType_GetModule() and PyType_GetModuleBydef() have it as well as the family. PyType_GetModuleBydef() actually misses about 10% on the telco benchmark.

If this patch is too OS specific, alternative ways to recover the PyType_GetModuleBydef() performance would be:

  • Make _PyType_HasFeature() an alias of PyType_HasFeature().
  • Use if-else in the loop, which needs to be checked after _PyType_GetModuleBydef2() is removed.

@neonene
Copy link
Contributor Author

neonene commented Sep 23, 2024

I'll check if it is good to to put PyType_GetBaseByToken() family above the PyType_GetModuleBydef() without the forward declarations.

UPDATE: The ugly order below improved the performance, but it was my silly idea to try to rely on unrelated functions.

 PyType_GetModuleState()
 
+get_base_by_token_recursive()
+get_base_by_token_from_mro()
+check_base_by_token()
+PyType_GetBaseByToken()
 
 PyType_GetModuleByDef()

@neonene neonene marked this pull request as draft September 24, 2024 08:07
@neonene
Copy link
Contributor Author

neonene commented Sep 24, 2024

I'll make an alternative PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants