Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 35.2k
gh-144145: Track nullness of properties in the Tier 2 JIT optimizer#144122
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
dfd99d77bb2c2020920fc2e9c5a65b4bd5f6a828d790e4800bf29c13b80bce1ec50130da398687c0c815873ca918bd0ff90a7cb9a2f8c2ceddbb9c59c91e86f74096ba7f00bfFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -16,6 +16,10 @@ extern "C" { | ||
| #define TY_ARENA_SIZE (UOP_MAX_TRACE_LENGTH * 5) | ||
| // Maximum descriptor mappings per object tracked symbolically | ||
| #define MAX_SYMBOLIC_DESCR_SIZE 16 | ||
| #define DESCR_ARENA_SIZE (MAX_SYMBOLIC_DESCR_SIZE * 100) | ||
| // Need extras for root frame and for overflow frame (see TRACE_STACK_PUSH()) | ||
| #define MAX_ABSTRACT_FRAME_DEPTH (16) | ||
| @@ -41,6 +45,7 @@ typedef enum _JitSymType { | ||
| JIT_SYM_TRUTHINESS_TAG = 9, | ||
| JIT_SYM_COMPACT_INT = 10, | ||
| JIT_SYM_PREDICATE_TAG = 11, | ||
| JIT_SYM_DESCR_TAG = 12, | ||
| } JitSymType; | ||
| typedef struct _jit_opt_known_class { | ||
| @@ -91,6 +96,31 @@ typedef struct { | ||
| uint8_t tag; | ||
| } JitOptCompactInt; | ||
| /* | ||
| Mapping from slot index or attribute offset to its symbolic value. | ||
| SAFETY: | ||
| This structure is used for both STORE_ATTR_SLOT and STORE_ATTR_INSTANCE_VALUE. | ||
| These two never appear on the same object type because: | ||
| __slots__ classes don't have Py_TPFLAGS_INLINE_VALUES | ||
| Therefore, there is no index collision between slot offsets and inline value offsets. | ||
| Note: | ||
| STORE_ATTR_WITH_HINT is NOT currently tracked. | ||
| If we want to track it in the future, we need to be careful about | ||
| potential index collisions with STORE_ATTR_INSTANCE_VALUE. | ||
| */ | ||
| typedef struct { | ||
| uint16_t slot_index; | ||
| uint16_t symbol; | ||
| } JitOptDescrMapping; | ||
cocolato marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| typedef struct _jit_opt_descr { | ||
| uint8_t tag; | ||
| uint8_t num_descrs; | ||
| uint16_t last_modified_index; // Index in out_buffer when this object was last modified | ||
| uint32_t type_version; | ||
| JitOptDescrMapping *descrs; | ||
| } JitOptDescrObject; | ||
| typedef union _jit_opt_symbol { | ||
| uint8_t tag; | ||
| JitOptKnownClass cls; | ||
| @@ -99,6 +129,7 @@ typedef union _jit_opt_symbol { | ||
| JitOptTuple tuple; | ||
| JitOptTruthiness truthiness; | ||
| JitOptCompactInt compact; | ||
| JitOptDescrObject descr; | ||
| JitOptPredicate predicate; | ||
| } JitOptSymbol; | ||
| @@ -128,6 +159,11 @@ typedef struct ty_arena { | ||
| JitOptSymbol arena[TY_ARENA_SIZE]; | ||
| } ty_arena; | ||
| typedef struct descr_arena { | ||
| int descr_curr_number; | ||
| int descr_max_number; | ||
| JitOptDescrMapping arena[DESCR_ARENA_SIZE]; | ||
| } descr_arena; | ||
| #ifdef __cplusplus | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it safe to use this for both normal objects and objects with
__slots__, or do we need a separate symbol?Basically I'm asking if it's possible for an object to both have STORE_ATTR_INSTANCE_VALUE and STORE_ATTR_SLOT, as this will cause a index collision between the slots and offset. It it safe also with
STORE_ATTR_WITH_HINT, as can that can mix withSTORE_ATTR_INSTANCE_VALUE?If you think of an answer, please let me know, and we can add it as a comment in the code. otherwise, this is kind of scary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two never appear on the same object type because:
STORE_ATTR_INSTANCE_VALUEneedsPy_TPFLAGS_MANAGED_DICTflag.Therefore, there is no index collision between slot offsets and inline value offsets.
cpython/Python/specialize.c
Lines 665 to 669 in 5f57f69
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
However, conflicts between
STORE_ATTR_INSTANCE_VALUEandSTORE_ATTR_WITH_HINTcan indeed occur. Perhaps a flag could be added to the index to distinguish between the two types?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we track
STORE_ATTR_WITH_HINTin this PR?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No lets ignore with hint for now.