-
Notifications
You must be signed in to change notification settings - Fork 377
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
Implement support for opt-in eagerly-serialized archetypes #8646
base: cmc/eager_partial_2_tagged_from_to_arrow
Are you sure you want to change the base?
Implement support for opt-in eagerly-serialized archetypes #8646
Conversation
Web viewer built successfully. If applicable, you should also test it:
Note: This comment is updated whenever you push a commit. |
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.
BEAUTIFUL ⭐⭐⭐⭐⭐
} | ||
}; | ||
|
||
// TODO(cmc): This goes away once all archetypes have been made eager. |
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.
nit: maybe link to the relevant issue here
|
||
let (_, unwrapped) = quote_field_type_from_typ(&field.typ, true); | ||
if unwrapped { | ||
// This was originally a vec/array! |
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.
This comment confused me for a bit. What you seem to mean is that this component is defines as being a many-component (as opposed to being a mono-component)?
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.
same below
let keypoint_ids = arrays_by_descr | ||
.get(&Self::descriptor_keypoint_ids()) | ||
.map(|array| { | ||
SerializedComponentBatch::new(array.clone(), Self::descriptor_keypoint_ids()) | ||
}); |
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.
It's so much shorter and beautiful ❤️
/// | ||
/// # Fallibility | ||
/// | ||
/// There are very few ways in which serialization can fail, all of which are very rare to hit |
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.
It would be good to at least mention the few failure cases (e.g. overflow i32)
This introduces a new
attr.rust.archetype_eager
codegen attribute. When toggled, the associated archetype will now only carry raw Arrow data around, and go through the new eager logging APIs.The attribute has been set on
Points3D
:Legacy and eagerly-serialized archetypes can co-exist, making it possible to migrate everything incrementally.
Archetype::{from_arrow, to_arrow}
and friends tag compliant #8644