-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
perf(ast): shrink large struct sizes #7019
Comments
Btw, I'm not sure if additional boxing is a breaking change for Wasm plugins. I need to look at rkyv source code. |
@djkoloski Sorry for the ping, but I can't check it using the source code. Does boxing a field change the memory layout? |
Yes, that will change rkyv's serialized format. You can fix this with a wrapper type that restores the original layout: use rkyv::{
with::{ArchiveWith, DeserializeWith, SerializeWith},
Archive, Deserialize, Fallible, Serialize,
};
pub struct InlineBox;
impl<F: Archive> ArchiveWith<Box<F>> for InlineBox {
type Archived = F::Archived;
type Resolver = F::Resolver;
#[inline]
unsafe fn resolve_with(
field: &Box<F>,
pos: usize,
resolver: Self::Resolver,
out: *mut Self::Archived,
) {
(**field).resolve(pos, resolver, out);
}
}
impl<F, S> SerializeWith<Box<F>, S> for InlineBox
where
F: Serialize<S>,
S: Fallible + ?Sized,
{
#[inline]
fn serialize_with(
field: &Box<F>,
serializer: &mut S,
) -> Result<Self::Resolver, S::Error> {
(**field).serialize(serializer)
}
}
impl<F, D> DeserializeWith<F::Archived, Box<F>, D> for InlineBox
where
F: Archive,
D: Fallible + ?Sized,
F::Archived: Deserialize<F, D>,
{
#[inline]
fn deserialize_with(
field: &F::Archived,
deserializer: &mut D,
) -> Result<Box<F>, D::Error> {
Ok(Box::new(field.deserialize(deserializer)?))
}
} You can use wrapper types to customize how fields are archived without affecting the original type: // Before
#[derive(Archive, Deserialize, Serialize)]
struct OldFoo {
big_array: [u32; 100],
}
// After
#[derive(Archive, Deserialize, Serialize)]
struct NewFoo {
#[with(InlineBox)]
big_array: Box<[u32; 100]>,
}
fn main() {
use core::mem::size_of;
use rkyv::Archived;
println!("OldFoo: {}", size_of::<OldFoo>());
println!("archived OldFoo: {}", size_of::<Archived<OldFoo>>());
println!("NewFoo: {}", size_of::<NewFoo>());
println!("archived NewFoo: {}", size_of::<Archived<NewFoo>>());
}
|
So we can box fields without breaking Wasm plugins. The only problem is that it's quite time-consuming... |
@kdy1 If you can box |
By looking at the great effort you put into the two PRs, I suggest to only box some of the larger fields and make |
I optimized |
This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you. |
Background
https://nnethercote.github.io/perf-book/type-sizes.html
Problem
I have gathered a list of large structs (> 64 bytes) inside
swc_ecma_ast
, by usingRUSTFLAGS=-Zprint-type-sizes
with the help of top-type-sizes:Top Type Sizes
And from my own memory profiling, at least
Expr
(104 bytes) should be reduced as much as possible, it has the most impact on the memory allocator.Stmt
needs some shrinking too because there are places where the whole array is cloned, which made the allocator to allocate for MBs of memory:swc/crates/swc_ecma_transforms_optimization/src/simplify/branch/mod.rs
Line 1293 in f32be0e
Solution
The best approach is to box all enum variants for all enum types, and the second best approach is to box at least some of the larger struct fields. for example
Expr
->TaggedTpl.tpl
,Arrow.body
.Changing the AST will break all downstream dependencies, so a strategy need to be selected.
Note: Changing the AST is a definite breaking change for WASM plugins, and needs to be documented in https://swc.rs/docs/plugin/selecting-swc-core
Related discussion: #6991
The text was updated successfully, but these errors were encountered: