Skip to content

Commit

Permalink
reduce allocations for commonly-generated identifiers
Browse files Browse the repository at this point in the history
  • Loading branch information
@hawkw
hawkw committed Mar 1, 2024
1 parent 65da60f commit bde9fc5
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 48 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
View file

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

1 change: 1 addition & 0 deletions Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ proc-macro2 = { version = "1.0.74" }
syn = { version = "2.0.52", features = ["full", "parsing", "extra-traits"] }
serde_with = { version = "3.6.1", features = ["macros"] }
prettyplease = "0.2.16"
once_cell = "1.19.0"

[workspace]
members = ["runtime"]
18 changes: 6 additions & 12 deletions src/client.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,14 +131,8 @@ fn client_stub_tokens(
// identifiers.
let argmap = {
let argnames = {
let args = op
.args
.keys()
.map(|name| quote::format_ident!("arg_{name}"));
let leases = op
.leases
.keys()
.map(|name| quote::format_ident!("arg_{name}"));
let args = op.args.keys().map(syntax::Name::arg_prefixed);
let leases = op.leases.keys().map(syntax::Name::arg_prefixed);
args.chain(leases)
};
let argvals = op.args.keys().chain(op.leases.keys());
Expand All @@ -155,7 +149,7 @@ fn client_stub_tokens(
let lease_validators =
op.leases.iter().filter_map(|(leasename, lease)| {
let n = lease.max_len?.get();
let argname = quote::format_ident!("arg_{leasename}");
let argname = leasename.arg_prefixed();
// Note: we're not generating a panic message in the client to
// save ROM space. If the user chases the line number into the
// client stub source file the error should be clear.
Expand Down Expand Up @@ -254,7 +248,7 @@ fn client_stub_tokens(
// Create instance of args struct from args.
let mk_arg_struct = {
let initializers = op.args.iter().map(|(argname, arg)| {
let arg_argname = quote::format_ident!("arg_{argname}");
let arg_argname = argname.arg_prefixed();
if arg.ty.is_bool() {
// Special case: we send booleans as non-zero u8, so that
// we can use them in Zerocopy situations
Expand Down Expand Up @@ -292,7 +286,7 @@ fn client_stub_tokens(
};

let send = {
let op_enum_name = quote::format_ident!("{iface_name}Operation");
let op_enum_name = iface_name.as_op_enum();
let buf = match op.encoding {
syntax::Encoding::Zerocopy => {
quote! { zerocopy::AsBytes::as_bytes(&args) }
Expand All @@ -312,7 +306,7 @@ fn client_stub_tokens(
syn::Ident::new(ctor, proc_macro2::Span::call_site());
let asbytes =
syn::Ident::new(asbytes, proc_macro2::Span::call_site());
let argname = quote::format_ident!("arg_{leasename}");
let argname = leasename.arg_prefixed();
quote! {
userlib::Lease::#ctor(zerocopy::AsBytes::#asbytes(#argname))
}
Expand Down
4 changes: 2 additions & 2 deletions src/common.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// file, You can obtain one at https://mozilla.org/MPL/2.0/.

use super::syntax;
use quote::{format_ident, quote};
use quote::quote;

pub fn generate_op_enum(iface: &syntax::Interface) -> proc_macro2::TokenStream {
let variants = iface.ops.keys().enumerate().map(|(idx, name)| {
Expand All @@ -12,7 +12,7 @@ pub fn generate_op_enum(iface: &syntax::Interface) -> proc_macro2::TokenStream {
#name = #val,
}
});
let name = format_ident!("{}Operation", iface.name);
let name = iface.name.as_op_enum();
quote! {
#[allow(non_camel_case_types)]
#[derive(Copy, Clone, Debug, Eq, PartialEq, userlib::FromPrimitive)]
Expand Down
43 changes: 27 additions & 16 deletions src/server.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ pub fn build_server_support(
build_restricted_server_support(source, stub_name, style, &BTreeMap::new())
}

// `Name` is only mutable as it contains `OnceCell`s, but they don't effect its
// `Hash`, `PartialEq`, `Eq`, `Ord`, or `PartialOrd` implementations. So, it can
// safely be used as a map key.
#[allow(clippy::mutable_key_type)]
pub fn build_restricted_server_support(
source: &str,
stub_name: &str,
Expand All @@ -44,6 +48,10 @@ pub fn build_restricted_server_support(
Ok(())
}

// `Name` is only mutable as it contains `OnceCell`s, but they don't effect its
// `Hash`, `PartialEq`, `Eq`, `Ord`, or `PartialOrd` implementations. So, it can
// safely be used as a map key.
#[allow(clippy::mutable_key_type)]
pub fn generate_restricted_server_support(
iface: &syntax::Interface,
style: ServerStyle,
Expand Down Expand Up @@ -109,7 +117,7 @@ pub fn generate_server_constants(iface: &syntax::Interface) -> TokenStream {
tokens
};
let reply_size = {
let const_name = format_ident!("{}_REPLY_SIZE", name.uppercase());
let const_name = name.as_reply_size();
let val = match &op.reply {
syntax::Reply::Result { ok, .. } => {
// This strategy only uses bytes for the OK side of the type,
Expand Down Expand Up @@ -194,7 +202,7 @@ pub fn generate_server_conversions(iface: &syntax::Interface) -> TokenStream {
syntax::RecvStrategy::FromBytes if arg.ty.is_bool() => {
// Special-case handling to send bools using a Zerocopy
// encoding strategy, for efficiency.
let ident = format_ident!("raw_{argname}");
let ident =argname.raw_prefixed();
need_args_impl = true;
quote! {
pub #ident: u8
Expand All @@ -207,7 +215,7 @@ pub fn generate_server_conversions(iface: &syntax::Interface) -> TokenStream {
syntax::RecvStrategy::FromPrimitive(ty)
| syntax::RecvStrategy::From(ty, _) => {
need_args_impl = true;
let ident = format_ident!("raw_{argname}");
let ident = argname.raw_prefixed();
quote! {
pub #ident: #ty
}
Expand All @@ -227,7 +235,7 @@ pub fn generate_server_conversions(iface: &syntax::Interface) -> TokenStream {
match &arg.recv {
syntax::RecvStrategy::FromPrimitive(ty) => {
let arg_ty = &arg.ty;
let raw_argname = format_ident!("raw_{argname}");
let raw_argname = argname.raw_prefixed();
let from_ty = format_ident!("from_{ty}");
quote! {
pub fn #argname(&self) -> Option<#arg_ty> {
Expand All @@ -238,14 +246,13 @@ pub fn generate_server_conversions(iface: &syntax::Interface) -> TokenStream {
syntax::RecvStrategy::FromBytes if arg.ty.is_bool() => {
// The only FromBytes type which also needs a decoder
// function is a `bool` encoded as a single `u8`

let raw_argname = format_ident!("raw_{argname}");
quote! {
pub fn #argname(&self) -> bool {
self.#raw_argname != 0
}
let raw_argname = argname.raw_prefixed();
quote! {
pub fn #argname(&self) -> bool {
self.#raw_argname != 0
}
}
}
_ => quote! {}
}
});
Expand Down Expand Up @@ -324,9 +331,9 @@ pub fn generate_server_conversions(iface: &syntax::Interface) -> TokenStream {
}

pub fn generate_server_op_impl(iface: &syntax::Interface) -> TokenStream {
let op_enum = format_ident!("{}Operation", iface.name);
let op_enum = iface.name.as_op_enum();
let max_reply_size_cases = iface.ops.keys().map(|opname| {
let reply_size = format_ident!("{}_REPLY_SIZE", opname.uppercase());
let reply_size = opname.as_reply_size();
quote! {
Self::#opname => #reply_size,
}
Expand Down Expand Up @@ -355,6 +362,10 @@ pub fn generate_server_op_impl(iface: &syntax::Interface) -> TokenStream {
}
}

// `Name` is only mutable as it contains `OnceCell`s, but they don't effect its
// `Hash`, `PartialEq`, `Eq`, `Ord`, or `PartialOrd` implementations. So, it can
// safely be used as a map key.
#[allow(clippy::mutable_key_type)]
pub fn generate_server_in_order_trait(
iface: &syntax::Interface,
allowed_callers: &BTreeMap<syntax::Name, Vec<usize>>,
Expand All @@ -374,7 +385,7 @@ pub fn generate_server_in_order_trait(
let trt = format_ident!("InOrder{iface_name}Impl");
let trait_def = generate_trait_def(iface, &trt);

let enum_name = format_ident!("{iface_name}Operation");
let enum_name = iface.name.as_op_enum();
let op_cases = iface.ops.iter().map(|(opname, op)| {
let check_allowed = if let Some(allowed_callers) = allowed_callers.get(opname) {
// With our current optimization settings and rustc/llvm version,
Expand Down Expand Up @@ -428,13 +439,13 @@ pub fn generate_server_in_order_trait(
}
},
syntax::RecvStrategy::From(_, None) => {
let name = format_ident!("raw_{argname}");
let name = argname.raw_prefixed();
quote! {
args.#name.into()
}
}
syntax::RecvStrategy::From(_, Some(f)) => {
let name = format_ident!("raw_{argname}");
let name = argname.raw_prefixed();
quote! {
#f(args.#name)
}
Expand Down Expand Up @@ -491,7 +502,7 @@ pub fn generate_server_in_order_trait(
userlib::sys_reply(rm.sender, 0, zerocopy::AsBytes::as_bytes(&val));
},
syntax::Encoding::Hubpack | syntax::Encoding::Ssmarshal => {
let reply_size = format_ident!("{}_REPLY_SIZE", opname.uppercase());
let reply_size = opname.as_reply_size();
let serializer = op.encoding.crate_name();
quote! {
let mut reply_buf = [0u8; #reply_size];
Expand Down
107 changes: 89 additions & 18 deletions src/syntax.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,32 +8,36 @@
//! parser, but then, McCarthy said the same thing about s-expressions.
use indexmap::IndexMap;
use once_cell::unsync::OnceCell;
use quote::TokenStreamExt;
use serde::{Deserialize, Serialize};
use serde_with::{DeserializeFromStr, SerializeDisplay};
use std::num::NonZeroU32;

/// An identifier.
#[derive(
Clone,
Debug,
Eq,
PartialEq,
Ord,
PartialOrd,
Hash,
SerializeDisplay,
DeserializeFromStr,
)]
#[derive(Debug, SerializeDisplay, DeserializeFromStr)]
pub struct Name {
pub ident: syn::Ident,
/// A bunch of code generation makes constants, which require an uppercase
/// version of identifiers. Thus, we cache this to reduce the number of
/// strings we allocate a bunch of times during codegen.
///
/// This is, admittedly, a kind of goofy microoptimization that probably
/// doesn't matter that much.

// A bunch of code generation uses input identifiers to construct other
// identifiers, such as prefixing or uppercasing, multiple times for the
// same identifier. Thus, we cache these to reduce the number of
// strings we allocate a bunch of times during codegen.
//
// This is, admittedly, a kind of goofy microoptimization that probably
// doesn't matter that much.
/// Cached uppercase version of the identifier, used for generating constants.
uppercase: String,
/// Cached version of the identifier with a `arg_` prefix, used for
/// generating argument names in generated code.
arg_name: OnceCell<syn::Ident>,
/// Cached version of the identifier as a `{uppercase}_REPLY_SIZE` constant.
reply_size: OnceCell<syn::Ident>,
/// Cached version of the identifier with a `raw_` prefix, used for
/// generating arguments which perform conversions.
raw_argname: OnceCell<syn::Ident>,
/// Cached version of the identifier as `{name}Operation`.
op_enum: OnceCell<syn::Ident>,
}

impl std::fmt::Display for Name {
Expand All @@ -47,7 +51,14 @@ impl std::str::FromStr for Name {
fn from_str(s: &str) -> Result<Self, Self::Err> {
let ident = syn::parse_str(s)?;
let uppercase = s.to_uppercase();
Ok(Self { ident, uppercase })
Ok(Self {
ident,
arg_name: OnceCell::new(),
reply_size: OnceCell::new(),
raw_argname: OnceCell::new(),
op_enum: OnceCell::new(),
uppercase,
})
}
}

Expand All @@ -67,10 +78,70 @@ impl quote::IdentFragment for Name {
}
}

impl PartialEq for Name {
fn eq(&self, other: &Self) -> bool {
self.ident == other.ident
}
}

impl Eq for Name {}

impl Ord for Name {
fn cmp(&self, other: &Self) -> std::cmp::Ordering {
self.ident.cmp(&other.ident)
}
}

impl PartialOrd for Name {
fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
Some(self.cmp(other))
}
}

impl std::hash::Hash for Name {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
self.ident.hash(state);
}
}

impl Clone for Name {
fn clone(&self) -> Self {
Self {
ident: self.ident.clone(),
uppercase: self.uppercase.clone(),
arg_name: OnceCell::new(),
reply_size: OnceCell::new(),
raw_argname: OnceCell::new(),
op_enum: OnceCell::new(),
}
}
}

impl Name {
pub(crate) fn uppercase(&self) -> &str {
&self.uppercase
}

pub(crate) fn arg_prefixed(&self) -> &syn::Ident {
self.arg_name
.get_or_init(|| quote::format_ident!("arg_{}", self))
}

pub(crate) fn as_reply_size(&self) -> &syn::Ident {
self.reply_size.get_or_init(|| {
quote::format_ident!("{}_REPLY_SIZE", self.uppercase)
})
}

pub(crate) fn raw_prefixed(&self) -> &syn::Ident {
self.raw_argname
.get_or_init(|| quote::format_ident!("raw_{self}"))
}

pub(crate) fn as_op_enum(&self) -> &syn::Ident {
self.op_enum
.get_or_init(|| quote::format_ident!("{self}Operation"))
}
}

/// Definition of an IPC interface.
Expand Down

0 comments on commit bde9fc5

Please sign in to comment.