Skip to content
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

chore: improve CompactZstd macro #13277

Merged
merged 3 commits into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions crates/primitives/src/receipt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ use reth_primitives_traits::receipt::ReceiptExt;
use serde::{Deserialize, Serialize};

use crate::TxType;
#[cfg(feature = "reth-codec")]
use reth_zstd_compressors::{RECEIPT_COMPRESSOR, RECEIPT_DECOMPRESSOR};

/// Retrieves gas spent by transactions as a vector of tuples (transaction index, gas used).
pub use reth_primitives_traits::receipt::gas_spent_by_transactions;
Expand All @@ -25,6 +23,10 @@ pub use reth_primitives_traits::receipt::gas_spent_by_transactions;
)]
#[cfg_attr(any(test, feature = "reth-codec"), derive(reth_codecs::CompactZstd))]
#[cfg_attr(any(test, feature = "reth-codec"), reth_codecs::add_arbitrary_tests)]
#[cfg_attr(any(test, feature = "reth-codec"), reth_zstd(
compressor = reth_zstd_compressors::RECEIPT_COMPRESSOR,
decompressor = reth_zstd_compressors::RECEIPT_DECOMPRESSOR
))]
#[rlp(trailing)]
pub struct Receipt {
/// Receipt type.
Expand Down
1 change: 0 additions & 1 deletion crates/prune/types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ workspace = true
reth-codecs.workspace = true

alloy-primitives.workspace = true
bytes.workspace = true
derive_more.workspace = true
modular-bitfield.workspace = true
serde.workspace = true
Expand Down
77 changes: 42 additions & 35 deletions crates/storage/codecs/derive/src/compact/generator.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Code generator for the `Compact` trait.

use super::*;
use crate::ZstdConfig;
use convert_case::{Case, Casing};
use syn::{Attribute, LitStr};

Expand All @@ -10,20 +11,20 @@ pub fn generate_from_to(
attrs: &[Attribute],
has_lifetime: bool,
fields: &FieldList,
is_zstd: bool,
zstd: Option<ZstdConfig>,
) -> TokenStream2 {
let flags = format_ident!("{ident}Flags");

let to_compact = generate_to_compact(fields, ident, is_zstd);
let from_compact = generate_from_compact(fields, ident, is_zstd);
let reth_codecs = parse_reth_codecs_path(attrs).unwrap();

let to_compact = generate_to_compact(fields, ident, zstd.clone(), &reth_codecs);
let from_compact = generate_from_compact(fields, ident, zstd);

let snake_case_ident = ident.to_string().to_case(Case::Snake);

let fuzz = format_ident!("fuzz_test_{snake_case_ident}");
let test = format_ident!("fuzz_{snake_case_ident}");

let reth_codecs = parse_reth_codecs_path(attrs).unwrap();

let lifetime = if has_lifetime {
quote! { 'a }
} else {
Expand Down Expand Up @@ -77,7 +78,7 @@ pub fn generate_from_to(
#fuzz_tests

#impl_compact {
fn to_compact<B>(&self, buf: &mut B) -> usize where B: bytes::BufMut + AsMut<[u8]> {
fn to_compact<B>(&self, buf: &mut B) -> usize where B: #reth_codecs::__private::bytes::BufMut + AsMut<[u8]> {
let mut flags = #flags::default();
let mut total_length = 0;
#(#to_compact)*
Expand All @@ -92,7 +93,11 @@ pub fn generate_from_to(
}

/// Generates code to implement the `Compact` trait method `to_compact`.
fn generate_from_compact(fields: &FieldList, ident: &Ident, is_zstd: bool) -> TokenStream2 {
fn generate_from_compact(
fields: &FieldList,
ident: &Ident,
zstd: Option<ZstdConfig>,
) -> TokenStream2 {
let mut lines = vec![];
let mut known_types =
vec!["B256", "Address", "Bloom", "Vec", "TxHash", "BlockHash", "FixedBytes"];
Expand Down Expand Up @@ -147,38 +152,41 @@ fn generate_from_compact(fields: &FieldList, ident: &Ident, is_zstd: bool) -> To
// If the type has compression support, then check the `__zstd` flag. Otherwise, use the default
// code branch. However, even if it's a type with compression support, not all values are
// to be compressed (thus the zstd flag). Ideally only the bigger ones.
is_zstd
.then(|| {
let decompressor = format_ident!("{}_DECOMPRESSOR", ident.to_string().to_uppercase());
quote! {
if flags.__zstd() != 0 {
#decompressor.with(|decompressor| {
let decompressor = &mut decompressor.borrow_mut();
let decompressed = decompressor.decompress(buf);
let mut original_buf = buf;

let mut buf: &[u8] = decompressed;
#(#lines)*
(obj, original_buf)
})
} else {
if let Some(zstd) = zstd {
let decompressor = zstd.decompressor;
quote! {
if flags.__zstd() != 0 {
#decompressor.with(|decompressor| {
let decompressor = &mut decompressor.borrow_mut();
let decompressed = decompressor.decompress(buf);
let mut original_buf = buf;

let mut buf: &[u8] = decompressed;
#(#lines)*
(obj, buf)
}
}
})
.unwrap_or_else(|| {
quote! {
(obj, original_buf)
})
} else {
#(#lines)*
(obj, buf)
}
})
}
} else {
quote! {
#(#lines)*
(obj, buf)
}
}
}

/// Generates code to implement the `Compact` trait method `from_compact`.
fn generate_to_compact(fields: &FieldList, ident: &Ident, is_zstd: bool) -> Vec<TokenStream2> {
fn generate_to_compact(
fields: &FieldList,
ident: &Ident,
zstd: Option<ZstdConfig>,
reth_codecs: &syn::Path,
) -> Vec<TokenStream2> {
let mut lines = vec![quote! {
let mut buffer = bytes::BytesMut::new();
let mut buffer = #reth_codecs::__private::bytes::BytesMut::new();
}];

let is_enum = fields.iter().any(|field| matches!(field, FieldTypes::EnumVariant(_)));
Expand All @@ -198,7 +206,7 @@ fn generate_to_compact(fields: &FieldList, ident: &Ident, is_zstd: bool) -> Vec<
// Just because a type supports compression, doesn't mean all its values are to be compressed.
// We skip the smaller ones, and thus require a flag` __zstd` to specify if this value is
// compressed or not.
if is_zstd {
if zstd.is_some() {
lines.push(quote! {
let mut zstd = buffer.len() > 7;
if zstd {
Expand All @@ -214,9 +222,8 @@ fn generate_to_compact(fields: &FieldList, ident: &Ident, is_zstd: bool) -> Vec<
buf.put_slice(&flags);
});

if is_zstd {
let compressor = format_ident!("{}_COMPRESSOR", ident.to_string().to_uppercase());

if let Some(zstd) = zstd {
let compressor = zstd.compressor;
lines.push(quote! {
if zstd {
#compressor.with(|compressor| {
Expand Down
10 changes: 6 additions & 4 deletions crates/storage/codecs/derive/src/compact/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ use flags::*;
mod structs;
use structs::*;

use crate::ZstdConfig;

// Helper Alias type
type IsCompact = bool;
// Helper Alias type
Expand All @@ -40,16 +42,16 @@ pub enum FieldTypes {
}

/// Derives the `Compact` trait and its from/to implementations.
pub fn derive(input: TokenStream, is_zstd: bool) -> TokenStream {
pub fn derive(input: TokenStream, zstd: Option<ZstdConfig>) -> TokenStream {
let mut output = quote! {};

let DeriveInput { ident, data, generics, attrs, .. } = parse_macro_input!(input);

let has_lifetime = has_lifetime(&generics);

let fields = get_fields(&data);
output.extend(generate_flag_struct(&ident, &attrs, has_lifetime, &fields, is_zstd));
output.extend(generate_from_to(&ident, &attrs, has_lifetime, &fields, is_zstd));
output.extend(generate_flag_struct(&ident, &attrs, has_lifetime, &fields, zstd.is_some()));
output.extend(generate_from_to(&ident, &attrs, has_lifetime, &fields, zstd));
output.into()
}

Expand Down Expand Up @@ -236,7 +238,7 @@ mod tests {
let DeriveInput { ident, data, attrs, .. } = parse2(f_struct).unwrap();
let fields = get_fields(&data);
output.extend(generate_flag_struct(&ident, &attrs, false, &fields, false));
output.extend(generate_from_to(&ident, &attrs, false, &fields, false));
output.extend(generate_from_to(&ident, &attrs, false, &fields, None));

// Expected output in a TokenStream format. Commas matter!
let should_output = quote! {
Expand Down
50 changes: 45 additions & 5 deletions crates/storage/codecs/derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ use syn::{
mod arbitrary;
mod compact;

#[derive(Clone)]
pub(crate) struct ZstdConfig {
compressor: syn::Path,
decompressor: syn::Path,
}

/// Derives the `Compact` trait for custom structs, optimizing serialization with a possible
/// bitflag struct.
///
Expand Down Expand Up @@ -51,15 +57,49 @@ mod compact;
/// efficient decoding.
#[proc_macro_derive(Compact, attributes(maybe_zero, reth_codecs))]
pub fn derive(input: TokenStream) -> TokenStream {
let is_zstd = false;
compact::derive(input, is_zstd)
compact::derive(input, None)
}

/// Adds `zstd` compression to derived [`Compact`].
#[proc_macro_derive(CompactZstd, attributes(maybe_zero, reth_codecs))]
#[proc_macro_derive(CompactZstd, attributes(maybe_zero, reth_codecs, reth_zstd))]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adding reth_zstd makes sense, this a lot smarter than adding it to reth_codecs because then we don't run into the issue I had on my pr

pub fn derive_zstd(input: TokenStream) -> TokenStream {
let is_zstd = true;
compact::derive(input, is_zstd)
let derive_input = {
let input = input.clone();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wonder if we need to do this here or if we can do this

let DeriveInput { ident, data, generics, attrs, .. } = parse_macro_input!(input);

and get rid of the clone?
unsure though, we don't use zsdt derive a lot so this seems fine anyway

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated compact::derive to just accept a pre-parsed DeriveInput, we don't need clone that way

parse_macro_input!(input as DeriveInput)
};

let mut compressor = None;
let mut decompressor = None;

for attr in derive_input.attrs {
if attr.path().is_ident("reth_zstd") {
if let Err(err) = attr.parse_nested_meta(|meta| {
if meta.path.is_ident("compressor") {
let value = meta.value()?;
let path: syn::Path = value.parse()?;
compressor = Some(path);
} else if meta.path.is_ident("decompressor") {
let value = meta.value()?;
let path: syn::Path = value.parse()?;
decompressor = Some(path);
} else {
return Err(meta.error("unsupported attribute"))
}
Ok(())
}) {
return err.to_compile_error().into()
}
}
}

let (Some(compressor), Some(decompressor)) = (compressor, decompressor) else {
return quote! {
compile_error!("missing compressor or decompressor attribute");
}
.into()
};

compact::derive(input, Some(ZstdConfig { compressor, decompressor }))
}

/// Generates tests for given type.
Expand Down
2 changes: 1 addition & 1 deletion crates/storage/codecs/src/private.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
pub use modular_bitfield;

pub use bytes::Buf;
pub use bytes::{self, Buf};
Loading