-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
[mypyc] Promotion to Value Type optimization #17932
base: master
Are you sure you want to change the base?
Conversation
Value types are a great feature, thanks! This seems to address mypyc/mypyc#841 -- what do you think? Since using a value type can change semantics (at least |
5471267
to
6868d34
Compare
6755c87
to
51379e2
Compare
Yes, it goes in that direction, however the implementation presented here do not cover the
For sure makes sense, I already updated the PR for using |
I don't think NamedTuple can be a value type, since it supports subclassing and subclasses can add settable attributes. The idea with
I don't think they anything like that, and besides dataclasses have reference semantics, even if they are frozen, and support subclassing. |
makes sense, I didnt saw that NamedTuples can be subclassed and add new mutable members |
@JukkaL can you review at this point. I think is ready due the tests I have made, let me know if you consider any other test. |
Can you merge master? It now includes the dunder changes. This is a big PR so it will take some time to review and test. I'll start looking into this early next week probably. |
d926eb6
to
4fde6f9
Compare
Done |
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.
Did a quick review pass, focusing on the semantics of value types (didn't look at implementation details carefully yet).
These features may not have tests (and some are probably not implemented):
If some of these are unclear, I can help. Also, implementing all of these in a single PR may make the PR too big for comfortable reviewing. It's okay to implement these in multiple PRs, if the functionality is behind a feature flag until complete. We have way of hiding features behind feature flags ( |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@JukkaL Hey, I have made some modifications there. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
@JukkaL any other idea of aspect to test? I have covered:
|
This PR introduce to mypyc the ability to generate efficient classes passed by value
unleashing deeper optimizations in the C generated code.
With this change mypyc will be finding immutable classes to be promoted as Value Types.
Value types are unboxed representation of classes that can be passed by value avoiding
dynamic memory allocations and replacing those with stack based values.
Should be reviewed and rebased after #17934
For example:
For this class
The add method will produce the code:
Note the values passed as structs directly and returning it directly too.
It has an impact similar to the RTuple but should work transparently with more flexible definitions due the class syntax.
It works similar to Value Types in C# (CLR) or structures in Swift.
It will allow the language be a more suitable option for performance centric tasks like video game engines or real time simulation computation.
Implementation details
It uses the vtable field to signal if any exception happen similar to the empty flag in RTuple.
A new RType called RInstanceValue is created to represent value types.
All semantics of boxed and unboxed values are preserved where (RInstanceValue is unboxed) and RInstance continues being (boxed).
In order to easily box values using the existing infrastructure, the type's setup method has been used and promoted to exported function.