gh-146099: Optimize _GUARD_CODE_VERSION+IP via function version symbols#146101
gh-146099: Optimize _GUARD_CODE_VERSION+IP via function version symbols#146101markshannon merged 5 commits intopython:mainfrom
Conversation
markshannon
left a comment
There was a problem hiding this comment.
Thanks for doing this, the general approach looks sound.
I've spotted one correctness issue, and suggested a few minor tweaks
Python/optimizer_bytecodes.c
Outdated
| PyCodeObject *co = get_current_code_object(ctx); | ||
| if (co->co_version == version) { | ||
| _Py_BloomFilter_Add(dependencies, co); | ||
| if (sym_get_func_version(ctx->frame->callable) != 0) { |
There was a problem hiding this comment.
| if (sym_get_func_version(ctx->frame->callable) != 0) { | |
| if (sym_get_func_version(ctx->frame->callable) == version) { |
If the version is different this guard should fail
Python/optimizer_symbols.c
Outdated
| | KNOWN_CLASS--+ | | | | ||
| | | | | | | PREDICATE RECORDED_VALUE(known type) | ||
| | | | INT* | | | | | ||
| | | | | FUNC_VERSION | | | <- Anything below this level has a known truthiness. |
There was a problem hiding this comment.
The comment says that anything below this line has known truthiness, and functions are always true.
Add a new line below the "Anything below this level has a known truthiness" line
Python/optimizer_symbols.c
Outdated
| printf("<v%u at %p>", sym->version.version, (void *)sym); | ||
| break; | ||
| case JIT_SYM_FUNC_VERSION_TAG: | ||
| printf("<fv%u at %p>", sym->func_version.func_version, (void *)sym); |
There was a problem hiding this comment.
| printf("<fv%u at %p>", sym->func_version.func_version, (void *)sym); | |
| printf("<function version=%u>", sym->func_version.func_version); |
Be a bit less cryptic (the pointer contains no useful information)
Python/optimizer_symbols.c
Outdated
| ((PyFunctionObject *)sym->recorded_value.value)->func_version != version) { | ||
| sym_set_bottom(ctx, sym); | ||
| return false; | ||
| } |
|
When you're done making the requested changes, leave the comment: |
|
I have made the requested changes; please review again. |
|
Thanks for making the requested changes! @markshannon: please review the changes made to this pull request. |
|
I can confirm this fixes somewhat the regression seen on x86_64 for base16 benchmark in the past few days:
|
* upstream/main: (1475 commits) Docs: replace all `datetime` imports with `import datetime as dt` (python#145640) pythongh-146153: Use `frozendict` in pure python fallback for `curses.has_key` (python#146154) pythongh-138234: clarify returncode behavior for subprocesses created with `shell=True` (python#138536) pythongh-140947: fix contextvars handling for server tasks in asyncio (python#141158) pythonGH-100108: Add async generators best practices section (python#141885) pythonGH-145667: Merge `GET_ITER` and `GET_YIELD_FROM_ITER` (pythonGH-146120) pythongh-146228: Better fork support in cached FastPath (python#146231) pythongh-146227: Fix wrong type in _Py_atomic_load_uint16 in pyatomic_std.h (pythongh-146229) pythongh-145980: Fix copy/paste mistake in binascii.c (python#146230) pythongh-146092: Raise MemoryError on allocation failure in _zoneinfo (python#146165) pythongh-91279: Note `SOURCE_DATE_EPOCH` support in `ZipFile.writestr()` doc (python#139396) pythongh-146196: Fix Undefined Behavior in _PyUnicodeWriter_WriteASCIIString() (python#146201) pythongh-143930: Reject leading dashes in webbrowser URLs pythongh-145916: Soft-deprecate ctypes.util.find_library (pythonGH-145919) pythongh-146205: Check the errno with != 0 in close impls in select module (python#146206) pythongh-146171: Fix nested AttributeError suggestions (python#146188) pythongh-146099: Optimize _GUARD_CODE_VERSION+IP via function version symbols (pythonGH-146101) pythongh-145980: Add support for alternative alphabets in the binascii module (pythonGH-145981) pythongh-145754: Update signature retrieval in unittest.mock to use forwardref annotation format (python#145756) pythongh-145177: Add emscripten run --test, uses test args from config.toml (python#146160) ...
Uh oh!
There was an error while loading. Please reload this page.