Skip to content

gh-146099: Optimize _GUARD_CODE_VERSION+IP via function version symbols#146101

Merged
markshannon merged 5 commits intopython:mainfrom
Fidget-Spinner:opt_guard_code_version_2
Mar 20, 2026
Merged

gh-146099: Optimize _GUARD_CODE_VERSION+IP via function version symbols#146101
markshannon merged 5 commits intopython:mainfrom
Fidget-Spinner:opt_guard_code_version_2

Conversation

@Fidget-Spinner
Copy link
Member

@Fidget-Spinner Fidget-Spinner commented Mar 18, 2026

Copy link
Member

@markshannon markshannon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this, the general approach looks sound.
I've spotted one correctness issue, and suggested a few minor tweaks

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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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

| KNOWN_CLASS--+ | | |
| | | | | | PREDICATE RECORDED_VALUE(known type)
| | | INT* | | | |
| | | | FUNC_VERSION | | | <- Anything below this level has a known truthiness.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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)

((PyFunctionObject *)sym->recorded_value.value)->func_version != version) {
sym_set_bottom(ctx, sym);
return false;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}
}

@bedevere-app
Copy link

bedevere-app bot commented Mar 19, 2026

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@Fidget-Spinner
Copy link
Member Author

I have made the requested changes; please review again.

@bedevere-app
Copy link

bedevere-app bot commented Mar 19, 2026

Thanks for making the requested changes!

@markshannon: please review the changes made to this pull request.

@bedevere-app bedevere-app bot requested a review from markshannon March 19, 2026 14:40
@Fidget-Spinner
Copy link
Member Author

Fidget-Spinner commented Mar 19, 2026

I can confirm this fixes somewhat the regression seen on x86_64 for base16 benchmark in the past few days:

base16_large: Mean +- std dev: [base64_main] 10.2 ms +- 0.2 ms -> [base64_fixed] 9.38 ms +- 0.17 ms: 1.08x faster

Copy link
Member

@markshannon markshannon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now. Thanks

@markshannon markshannon merged commit acfb452 into python:main Mar 20, 2026
69 of 70 checks passed
@Fidget-Spinner Fidget-Spinner deleted the opt_guard_code_version_2 branch March 20, 2026 12:40
CuriousLearner added a commit to CuriousLearner/cpython that referenced this pull request Mar 22, 2026
* 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)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants