-
Notifications
You must be signed in to change notification settings - Fork 107
Performance_fix: Move serialize to Rust #1305
Performance_fix: Move serialize to Rust #1305
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main-v0.13.0 #1305 +/- ##
================================================
- Coverage 67.01% 65.12% -1.90%
================================================
Files 53 53
Lines 6885 7234 +349
Branches 6885 7234 +349
================================================
+ Hits 4614 4711 +97
- Misses 1859 2116 +257
+ Partials 412 407 -5 ☔ View full report in Codecov by Sentry. |
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.
Reviewable status: 0 of 7 files reviewed, all discussions resolved (waiting on @elintul and @Yoni-Starkware)
crates/native_blockifier/src/py_utils.rs
line 15 at r1 (raw file):
#[derive(Clone, Copy, Default, Eq, FromPyObject, Hash, PartialEq, Debug, Serialize)] pub struct PyFelt(#[pyo3(from_py_with = "int_to_stark_felt")] pub StarkFelt);
Does this mean that it already knows to map it into an int
on serialize?
Code quote:
#[derive(Clone, Copy, Default, Eq, FromPyObject, Hash, PartialEq, Debug, Serialize)]
pub struct PyFelt(#[pyo3(from_py_with = "int_to_stark_felt")] pub StarkFelt);
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.
Good.
While this is being tested, if you want @elintul, we can check if this can be done solely with derive serde::Serialize
in those structs + some serde customizations for specific serialization logic, i believe it can.
Reviewable status: 0 of 7 files reviewed, all discussions resolved (waiting on @elintul and @Yoni-Starkware)
3f7135d
to
2f112b5
Compare
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.
Also, you can consult with DanB team, they do stuff like this efficiently.
Reviewable status: 0 of 7 files reviewed, all discussions resolved (waiting on @elintul)
94fda43
to
d26a911
Compare
d26a911
to
a7b812b
Compare
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 this PR still relevant given your other PR that uses Serialize?
Reviewable status: 0 of 7 files reviewed, all discussions resolved (waiting on @elintul)
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.
Reviewable status: 0 of 7 files reviewed, 1 unresolved discussion (waiting on @elintul)
a discussion (no related file):
I will close this PR., as we've decide to change the Python class instead.
Python PR: https://github.com/starkware-industries/starkware/pull/33327
Rust PR: #1315
This change is