gh-121263: Macro-ify most stackref functions for MSVC#121270
gh-121263: Macro-ify most stackref functions for MSVC#121270Fidget-Spinner merged 1 commit intopython:mainfrom
Conversation
|
cc @mdboom |
|
Kicking off a benchmarking run now... |
|
The raw benchmaring results are here. This PR makes things 5% faster on 64-bit Windows, 7% faster on 32-bit Windows, and no measurable change on x86_64 Linux. The important comparison, however, is whether this change counteracts all of the negative effects of #118450. Comparing directly to commit d611c4c (the commit immediately before #118450 landed), shows the same amount of speedup 5% faster on 64-bit Windows, 7% faster on 32-bit Windows, and no measurable change on x86_64 Linux. So this PR is effective and seems to make up for the regression. However... if we could hold off on merging it until we confirm one other thing. @zooba reminded me that we still have a pragma "hack" to disable optimization on MSVC for the main eval loop. This was introduced to work around a compiler crash when the main eval loop function got extremely large (when the Tier 2 and Tier 2 interpreters were merged together). Of course, this hack is no longer needed for non-JIT builds, since they no longer include the Tier 2 interpreter. I am doing another benchmarking run just removing the hack to see if it also fixes the performance issue -- if we can do that and use inline functions rather than macros, that seems preferable. EDIT: Added link to change that removed Tier 2 interpreter from default builds. |
|
Thanks Mike for the attempt! Sadly, removing the pragma doesn't seem to speed up anything: As the reported benchmark results are good, and this PR seems to make up for all the perf loss, I will merge this PR, to restore performance of Windows builds. |
…-121270) Macro-ify most stackref functions for MSVC
…-121270) Macro-ify most stackref functions for MSVC
…ythonGH-121270)" This reverts commit 722229e.
Uh oh!
There was an error while loading. Please reload this page.