-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Use 1 byte per type/symbol tag #19735
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
Conversation
This comment has been minimized.
This comment has been minimized.
mypyc/primitives/misc_ops.py
Outdated
) | ||
|
||
function_op( | ||
name="native_internal.write_native_internal", |
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 the name be ...write_tag
, similar to the above ones?
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.
Oops, it looks like some Ctrl+R went wrong :-) Will fix it.
mypyc/primitives/misc_ops.py
Outdated
name="native_internal.write_native_internal", | ||
arg_types=[object_rprimitive, int_rprimitive], | ||
return_type=none_rprimitive, | ||
c_function_name="write_native_internal_internal", |
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 this be write_tag_internal
?
mypyc/primitives/misc_ops.py
Outdated
) | ||
|
||
function_op( | ||
name="native_internal.read_native_internal", |
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.
Similar to above.
mypyc/primitives/misc_ops.py
Outdated
name="native_internal.read_native_internal", | ||
arg_types=[object_rprimitive], | ||
return_type=int_rprimitive, | ||
c_function_name="read_native_internal_internal", |
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.
Similar to above.
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
As discussed offline, I think that this is a good idea (if it doesn't cause any trouble). I think it's okay to do this in a follow-up PR, but it's up to you. |
OK, I will make a separate PR for |
This is a small incremental improvement for fixed format cache. I am adding a dedicated write/read functions for tags (i.e. integers in 0-255 range). I propose to exclusively use these functions for type tags (hence the name), and still use regular
write_int()
/read_int()
for integers that are "accidentally small" (like argument kinds etc). In a separate PR I will change regularint
format to be more progressive (e.g. only use 1 byte if an integer happens to be small). I also change the terminology from "marker" to "tag", as this is a more common name for this concept.Note we can probably use
mypy_extensions.u8
for type tags. If there is a desire for this, I can switch to it (either in this or a separate PR).