From b4da52f7fb656d63f69646147a8ed01d82906e05 Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Fri, 20 Mar 2026 22:15:54 +0000 Subject: [PATCH] perf(ast_tools): reduce data passing between functions (#20579) Refactor `ast_tools`. Simplify code by storing `is_meta` flag in `StructSkeleton` and `EnumSkeleton` types, instead of passing around a `bool` separately. --- tasks/ast_tools/src/parse/load.rs | 62 +++++++++++---------------- tasks/ast_tools/src/parse/mod.rs | 9 ++-- tasks/ast_tools/src/parse/parse.rs | 4 +- tasks/ast_tools/src/parse/skeleton.rs | 27 +++++++++++- 4 files changed, 55 insertions(+), 47 deletions(-) diff --git a/tasks/ast_tools/src/parse/load.rs b/tasks/ast_tools/src/parse/load.rs index b5e6e0af52234..3b6d733d47c14 100644 --- a/tasks/ast_tools/src/parse/load.rs +++ b/tasks/ast_tools/src/parse/load.rs @@ -22,54 +22,39 @@ use super::{ /// * Inherits of enums wrapped in `inherit_variants!` macro. /// /// Returns a list of [`Skeleton`]s found in the file. -/// Each entry is `(type_name, skeleton, is_meta)`. /// /// This is the bare minimum to be able to "link up" types to each other in next pass. -pub fn load_file( - file_id: FileId, - file_path: &str, - root_path: &Path, -) -> Vec<(String, Skeleton, bool)> { +pub fn load_file(file_id: FileId, file_path: &str, root_path: &Path) -> Vec { let content = fs::read_to_string(root_path.join(file_path)).unwrap(); let file = parse_file(content.as_str()).unwrap(); + file.items.into_iter().filter_map(|item| parse_item(item, file_id)).collect() +} - let mut results = Vec::new(); - for item in file.items { - let entry = match item { - Item::Struct(item) => { - let Some((skeleton, is_meta)) = parse_struct(item, file_id) else { continue }; - (skeleton.name.clone(), Skeleton::Struct(skeleton), is_meta) - } - Item::Enum(item) => { - let Some((skeleton, is_meta)) = parse_enum(item, file_id) else { continue }; - (skeleton.name.clone(), Skeleton::Enum(skeleton), is_meta) - } - Item::Macro(item) => { - let Some((name, skeleton, is_meta)) = parse_macro(&item, file_id) else { continue }; - (name, skeleton, is_meta) - } - _ => continue, - }; - results.push(entry); +fn parse_item(item: Item, file_id: FileId) -> Option { + match item { + Item::Struct(item) => parse_struct(item, file_id), + Item::Enum(item) => parse_enum(item, file_id), + Item::Macro(item) => parse_macro(&item, file_id), + _ => None, } - results } -fn parse_struct(item: ItemStruct, file_id: FileId) -> Option<(StructSkeleton, /* is_meta */ bool)> { +fn parse_struct(item: ItemStruct, file_id: FileId) -> Option { let (name, is_foreign, is_meta) = get_type_name(&item.attrs, &item.ident)?; - Some((StructSkeleton { name, is_foreign, file_id, item }, is_meta)) + let skeleton = StructSkeleton { name, file_id, is_foreign, is_meta, item }; + Some(Skeleton::Struct(skeleton)) } -fn parse_enum(item: ItemEnum, file_id: FileId) -> Option<(EnumSkeleton, /* is_meta */ bool)> { +fn parse_enum(item: ItemEnum, file_id: FileId) -> Option { let (name, is_foreign, is_meta) = get_type_name(&item.attrs, &item.ident)?; - Some((EnumSkeleton { name, is_foreign, file_id, item, inherits: vec![] }, is_meta)) + let skeleton = EnumSkeleton { name, file_id, is_foreign, is_meta, item, inherits: vec![] }; + Some(Skeleton::Enum(skeleton)) } -fn parse_macro(item: &ItemMacro, file_id: FileId) -> Option<(String, Skeleton, bool)> { +fn parse_macro(item: &ItemMacro, file_id: FileId) -> Option { if item.mac.path.is_ident("inherit_variants") { - let skeleton = parse_inherit_variants_macro(item, file_id); - return Some((skeleton.name.clone(), Skeleton::Enum(skeleton), false)); + return Some(parse_inherit_variants_macro(item, file_id)); } if item.mac.path.is_ident("define_nonmax_u32_index_type") || item.mac.path.is_ident("define_index_type") @@ -79,7 +64,7 @@ fn parse_macro(item: &ItemMacro, file_id: FileId) -> Option<(String, Skeleton, b None } -fn parse_inherit_variants_macro(item: &ItemMacro, file_id: FileId) -> EnumSkeleton { +fn parse_inherit_variants_macro(item: &ItemMacro, file_id: FileId) -> Skeleton { item.mac .parse_body_with(|input: &ParseBuffer| { // Because of `@inherit`s we can't use the actual `ItemEnum` parse. @@ -127,12 +112,14 @@ fn parse_inherit_variants_macro(item: &ItemMacro, file_id: FileId) -> EnumSkelet } let item = ItemEnum { attrs, vis, enum_token, ident, generics, brace_token, variants }; - Ok(EnumSkeleton { name, is_foreign: false, file_id, item, inherits }) + let skeleton = + EnumSkeleton { name, file_id, is_foreign: false, is_meta: false, item, inherits }; + Ok(Skeleton::Enum(skeleton)) }) .expect("Failed to parse contents of `inherit_variants!` macro") } -fn parse_index_type_macro(item: &ItemMacro, file_id: FileId) -> Option<(String, Skeleton, bool)> { +fn parse_index_type_macro(item: &ItemMacro, file_id: FileId) -> Option { item.mac .parse_body_with(|input: &ParseBuffer| { let attrs = input.call(Attribute::parse_outer)?; @@ -174,9 +161,8 @@ fn parse_index_type_macro(item: &ItemMacro, file_id: FileId) -> Option<(String, semi_token: None, }; - let skeleton = - StructSkeleton { name: name.clone(), is_foreign, file_id, item: item_struct }; - Ok(Some((name, Skeleton::Struct(skeleton), is_meta))) + let skeleton = StructSkeleton { name, file_id, is_foreign, is_meta, item: item_struct }; + Ok(Some(Skeleton::Struct(skeleton))) }) .ok()? } diff --git a/tasks/ast_tools/src/parse/mod.rs b/tasks/ast_tools/src/parse/mod.rs index d68585623e979..abc6aac9713d4 100644 --- a/tasks/ast_tools/src/parse/mod.rs +++ b/tasks/ast_tools/src/parse/mod.rs @@ -95,15 +95,14 @@ pub fn parse_files(file_paths: &[String], codegen: &Codegen) -> Schema { let mut files = IndexVec::new(); for AssertSend((file_path, file_skeletons)) in results { - for (name, skeleton, is_meta) in file_skeletons { - let (names, skeletons) = if is_meta { + for skeleton in file_skeletons { + let (names, skeletons) = if skeleton.is_meta() { (&mut meta_names, &mut meta_skeletons) } else { (&mut type_names, &mut type_skeletons) }; - - let (index, is_new) = names.insert_full(name); - assert!(is_new, "2 types with same name: {}", names.get_index(index).unwrap()); + let is_new = names.insert(skeleton.name().to_string()); + assert!(is_new, "2 types with same name: {}", skeleton.name()); skeletons.push(skeleton); } files.push(File::new(file_path)); diff --git a/tasks/ast_tools/src/parse/parse.rs b/tasks/ast_tools/src/parse/parse.rs index f881dbc14cab5..fb6a30bf254a9 100644 --- a/tasks/ast_tools/src/parse/parse.rs +++ b/tasks/ast_tools/src/parse/parse.rs @@ -303,7 +303,7 @@ impl<'c> Parser<'c> { /// Parse [`StructSkeleton`] to yield a [`TypeDef`]. fn parse_struct(&mut self, type_id: TypeId, skeleton: StructSkeleton) -> TypeDef { - let StructSkeleton { name, item, is_foreign, file_id } = skeleton; + let StructSkeleton { name, file_id, is_foreign, item, .. } = skeleton; let has_lifetime = check_generics(&item.generics, &name); let fields = self.parse_fields(&item.fields); let visibility = convert_visibility(&item.vis); @@ -381,7 +381,7 @@ impl<'c> Parser<'c> { /// Parse [`EnumSkeleton`] to yield a [`TypeDef`]. fn parse_enum(&mut self, type_id: TypeId, skeleton: EnumSkeleton) -> TypeDef { - let EnumSkeleton { name, item, inherits, is_foreign, file_id } = skeleton; + let EnumSkeleton { name, file_id, is_foreign, item, inherits, .. } = skeleton; let has_lifetime = check_generics(&item.generics, &name); let variants = item.variants.iter().map(|variant| self.parse_variant(variant)).collect(); let inherits = inherits.into_iter().map(|name| self.type_id(&name)).collect(); diff --git a/tasks/ast_tools/src/parse/skeleton.rs b/tasks/ast_tools/src/parse/skeleton.rs index 9a3eb9aa89917..bcb943f365fdf 100644 --- a/tasks/ast_tools/src/parse/skeleton.rs +++ b/tasks/ast_tools/src/parse/skeleton.rs @@ -15,19 +15,42 @@ pub enum Skeleton { Enum(EnumSkeleton), } +impl Skeleton { + pub fn name(&self) -> &str { + match self { + Self::Struct(s) => &s.name, + Self::Enum(e) => &e.name, + } + } + + pub fn is_meta(&self) -> bool { + match self { + Self::Struct(s) => s.is_meta, + Self::Enum(e) => e.is_meta, + } + } +} + +// These 2 structs are both `#[repr(C)]` so that `name` and `is_meta` are in same position in both, +// making the 2 methods above cheaper + +#[repr(C)] #[derive(Debug)] pub struct StructSkeleton { pub name: String, - pub is_foreign: bool, pub file_id: FileId, + pub is_foreign: bool, + pub is_meta: bool, pub item: ItemStruct, } +#[repr(C)] #[derive(Debug)] pub struct EnumSkeleton { pub name: String, - pub is_foreign: bool, pub file_id: FileId, + pub is_foreign: bool, + pub is_meta: bool, pub item: ItemEnum, pub inherits: Vec, }