8000 gh-119258: Eliminate Type Guards in Tier 2 Optimizer with Watcher by saulshanabrook · Pull Request #119365 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

gh-119258: Eliminate Type Guards in Tier 2 Optimizer with Watcher #119365

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.

Sign up for GitHub

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

Merged
merged 42 commits into from
Jun 8, 2024
Merged

gh-119258: Eliminate Type Guards in Tier 2 Optimizer with Watcher #119365

Changes from 1 commit
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
e7b7d5d
Add type version and offset to optimizer header
saulshanabrook May 20, 2024
c902276
Start adding type version
saulshanabrook May 20, 2024
89e3649
Add type version functions and struct initialization
saulshanabrook May 20, 2024
b8525be
fix typo
saulshanabrook May 20, 2024
4bd1608
Add failing test case
saulshanabrook May 20, 2024
ddbfd94
breakpoints and prints
dpdani May 20, 2024
f22130b
Add function definitions
saulshanabrook May 20, 2024
77bc293
wooohoooo
dpdani May 20, 2024
a2ebc49
unprint
dpdani May 20, 2024
e982837
Remove generated printf
saulshanabrook May 20, 2024
0e7e7eb
Remove test breakpoints
saulshanabrook May 20, 2024
4060ab8
📜🤖 Added by blurb_it.
blurb-it[bot] May 20, 2024
1520928
Revert "📜🤖 Added by blurb_it."
saulshanabrook May 20, 2024
5e0792b
Use global type watcher to invalidate type versions
saulshanabrook May 21, 2024
fd979f0
Revert formatting changes to tests
saulshanabrook May 21, 2024
5da1a40
Add additional test
saulshanabrook May 21, 2024
84860ea
Remove printf and make slightly more threadsafe
saulshanabrook May 21, 2024
c12bb90
fix typo
saulshanabrook May 21, 2024
53ab262
Update type watcher ID in test bc we know have a default one at 0
saulshanabrook May 22, 2024
88e2e59
Fix panic and add test for it
saulshanabrook May 22, 2024
1eabca9
add manual cast
saulshanabrook May 22, 2024
1f7cc74
Properly fixed the frame creation with discussion with Ken Jin
saulshanabrook May 22, 2024
f892ea7
Change type_assign_specific_version_unsafe to use safe setting
saulshanabrook May 23, 2024
06ed18f
📜🤖 Added by blurb_it.
blurb-it[bot] May 23, 2024
1343571
Fix more places where tp_version_tag is being set manually
brandtbucher May 23, 2024
2ee5126
Merge branch 'main' into optimizer-type-version-watcher
brandtbucher May 23, 2024
37aeea5
_PyType_ClearCodeByVersion isn't necessary
brandtbucher May 23, 2024
9d6171d
Merge branch 'main' into optimizer-type-version-watcher
Fidget-Spinner May 28, 2024
545c40b
Merge branch 'main' into optimizer-type-version-watcher
saulshanabrook May 29, 2024
c4436eb
Rename typ_version to type_version
saulshanabrook Jun 4, 2024
0d0c647
Change type of type_version to unsigned int
saulshanabrook Jun 4, 2024
9b80acb
C formatting fix
saulshanabrook Jun 4, 2024
6ddb1e7
Rephrase test names
saulshanabrook Jun 4, 2024
9d3f914
Rephrase comments
saulshanabrook Jun 4, 2024
3e0a1e7
Make existing inline test clear
saulshanabrook Jun 4, 2024
3420a42
Add a test for escaping behavior, but allow it to fail for now
saulshanabrook Jun 4, 2024
6886b36
Verify that expected type version is non zero
saulshanabrook Jun 4, 2024
376bb5e
Fix typo
saulshanabrook Jun 4, 2024
e025cc7
Assert pytype watch never errors
saulshanabrook Jun 4, 2024
88f2d89
Fix assertion
saulshanabrook Jun 4, 2024
82bc177
Remove unnecessary assert
saulshanabrook Jun 4, 2024
bdf0a4a
Handle if type versions are different
saulshanabrook Jun 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix typo
  • Loading branch information
@saulshanabrook
saulshanabrook committed Jun 4, 2024
commit 376bb5e75769f8441b172a9f85c427d3a4418642
2 changes: 1 addition & 1 deletion Python/optimizer_bytecodes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ dummy_func(void) {
if (sym_matches_type_version(owner, type_version)) {
Copy link
Member

Choose a reason for hiding this comment

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

Staring at this a bit harder, it seems like there are a couple of different cases that we want to distinguish:

  1. the symbol's version is 0, the expected version is 0
  2. the symbol's version is nonzero, the expected version is 0
  3. the symbol's version is 0, the expected version is nonzero
  4. the symbol's version is nonzero, the expected version is nonzero, both match
  5. the symbol's version is nonzero, the expected version is nonzero, both don't match

Cases 1 and 2 should be impossible, and we can assert(type_version) above this line.

Case 3 is what we're correctly handling in the else currently.

Case 4 is what we're correctly handling in the if currently.

Case 5 should result in narrowing owner to the "bottom" type, right? If so, this should probably happen in sym_set_type_version, which is called on line 127.

@Fidget-Spinner or @markshannon, can you confirm that my understanding is correct here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for thinking through all of these. For case 5, I am unclear how/when this could happen. This would only occur if _CHECK_TYPE_VERSION was ommitted twice, the first time with one type version, and then the second time with a different type version as an arg, but for the same symbolic value.

I am a little fuzzy on how these versions are generated. Are they captured based on the current type version of the type when it's generating the opcodes? If so, how could it be two different values?

If there is an example/test case here that would be helpful. Also is it possible to see in the test runner the additional args passed to each op, like the type_version?

Copy link
Member

Choose a reason for hiding this comment

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

Are they captured based on the current type version of the type when it's generating the opcodes? If so, how could it be two different values?

Yup the type version is the one at the time of specialization (bytecode rewriting). So if it just so happens that for example, a re-specialization is happening, the bytecode might have out of sync versions in their cache entries, leading to the scenario Brandt said in 5.

In general, we should hit the bottom if we're trying to set a type on something that is contradictory. E.g. if we're setting a type version on something that already has a type version. That is a contradiction. You can see an example test here

cpython/Python/optimizer_symbols.c

Line 484 in 82bc177

_Py_uop_sym_set_type(ctx, sym, &PyFloat_Type); // Should make it bottom

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense. I changed it so that it would set it to bottom if they are different versions (and not add the watcher b/c it must have already been added then).

After this change, I wonder if the logic would be clearer if it was all just in this block instead of in the get/set helper functions.

REPLACE_OP(this_instr, _NOP, 0, 0);
} else {
// add watcher so that whenever the type changes we invalide this
// add watcher so that whenever the type changes we invalidate this
PyTypeObject *type = _PyType_LookupByVersion(type_version);
// if the type is null, it was not found in the cache (there was a conflict)
// with the key, in which case we can't trust the version
Expand Down
0