forestia/bevy
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Internalize BevyManifest logic. Switch to RwLock (#18263)

Browse Source 
# Objective

Fixes #18103

#17330 introduced a significant compile time performance regression
(affects normal builds, clippy, and Rust Analyzer). While it did fix the
type-resolution bug (and the general approach there is still our best
known solution to the problem that doesn't involve [significant
maintenance
overhead](https://github.com/bevyengine/bevy/issues/18103#issuecomment-2702724676)),
the changes had a couple of issues:

1. It used a Mutex, which poses a significant threat to parallelization.
2. It externalized existing, relatively simple, performance critical
Bevy code to a crate outside of our control. I am not comfortable doing
that for cases like this. Going forward @bevyengine/maintainer-team
should be much stricter about this.
3. There were a number of other areas that introduced complexity and
overhead that I consider unnecessary for our use case. On a case by case
basis, if we encounter a need for more capabilities we can add them (and
weigh them against the cost of doing so).

## Solution

1. I moved us back to our original code as a baseline
2. I selectively ported over the minimal changes required to fix the
type resolution bug
3. I swapped `Mutex<BTreeMap<PathBuf, &'static Mutex<CargoManifest>>>`
for `RwLock<BTreeMap<PathBuf, CargoManifest>>`. Note that I used the
`parking_lot` RwLock because it has a mapping API that enables us to
return mapped guards.
This commit is contained in:
Carter Anderson 2025-03-11 17:46:01 -07:00 committed by GitHub
parent ecccd57417
commit e21dfe81ce
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 106 additions and 80 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
crates/bevy_encase_derive/src/lib.rs
View File

@ -14,7 +14,7 @@ const ENCASE: &str = "encase";
fn bevy_encase_path() -> syn::Path {
let bevy_manifest = BevyManifest::shared();
bevy_manifest
.get_subcrate("render")
.maybe_get_path("bevy_render")
.map(|bevy_render_path| {
let mut segments = bevy_render_path.segments;
segments.push(BevyManifest::parse_str("render_resource"));
@ -31,7 +31,7 @@ fn bevy_encase_path() -> syn::Path {
segments,
}
})
.unwrap_or_else(|_err| bevy_manifest.get_path(ENCASE))
.unwrap_or_else(|| bevy_manifest.get_path(ENCASE))
}
implement!(bevy_encase_path());

5
crates/bevy_macro_utils/Cargo.toml
View File

@ -12,7 +12,10 @@ keywords = ["bevy"]
syn = "2.0"
quote = "1.0"
proc-macro2 = "1.0"
cargo-manifest-proc-macros = "0.3.4"
toml_edit = { version = "0.22.7", default-features = false, features = [
"parse",
] }
parking_lot = { version = "0.12" }
[lints]
workspace = true

149
crates/bevy_macro_utils/src/bevy_manifest.rs
View File

@ -1,54 +1,111 @@
extern crate proc_macro;
use std::sync::MutexGuard;
use cargo_manifest_proc_macros::{
CargoManifest, CrateReExportingPolicy, KnownReExportingCrate, PathPiece,
TryResolveCratePathError,
};
use alloc::collections::BTreeMap;
use parking_lot::{lock_api::RwLockReadGuard, MappedRwLockReadGuard, RwLock, RwLockWriteGuard};
use proc_macro::TokenStream;
use std::{
env,
path::{Path, PathBuf},
time::SystemTime,
};
use toml_edit::{DocumentMut, Item};
struct BevyReExportingPolicy;
impl CrateReExportingPolicy for BevyReExportingPolicy {
fn get_re_exported_crate_path(&self, crate_name: &str) -> Option<PathPiece> {
crate_name.strip_prefix("bevy_").map(|s| {
let mut path = PathPiece::new();
path.push(syn::parse_str::<syn::PathSegment>(s).unwrap());
path
})
}
/// The path to the `Cargo.toml` file for the Bevy project.
#[derive(Debug)]
pub struct BevyManifest {
manifest: DocumentMut,
modified_time: SystemTime,
}
const BEVY: &str = "bevy";
const KNOWN_RE_EXPORTING_CRATE_BEVY: KnownReExportingCrate = KnownReExportingCrate {
re_exporting_crate_package_name: BEVY,
crate_re_exporting_policy: &BevyReExportingPolicy {},
};
const ALL_KNOWN_RE_EXPORTING_CRATES: &[&KnownReExportingCrate] = &[&KNOWN_RE_EXPORTING_CRATE_BEVY];
/// The path to the `Cargo.toml` file for the Bevy project.
pub struct BevyManifest(MutexGuard<'static, CargoManifest>);
impl BevyManifest {
/// Returns a global shared instance of the [`BevyManifest`] struct.
pub fn shared() -> Self {
Self(CargoManifest::shared())
pub fn shared() -> MappedRwLockReadGuard<'static, BevyManifest> {
static MANIFESTS: RwLock<BTreeMap<PathBuf, BevyManifest>> = RwLock::new(BTreeMap::new());
let manifest_path = Self::get_manifest_path();
let modified_time = Self::get_manifest_modified_time(&manifest_path)
.expect("The Cargo.toml should have a modified time");
if let Ok(manifest) =
RwLockReadGuard::try_map(MANIFESTS.read(), |manifests| manifests.get(&manifest_path))
{
if manifest.modified_time == modified_time {
return manifest;
}
}
let mut manifests = MANIFESTS.write();
manifests.insert(
manifest_path.clone(),
BevyManifest {
manifest: Self::read_manifest(&manifest_path),
modified_time,
},
);
RwLockReadGuard::map(RwLockWriteGuard::downgrade(manifests), |manifests| {
manifests.get(&manifest_path).unwrap()
})
}
fn get_manifest_path() -> PathBuf {
env::var_os("CARGO_MANIFEST_DIR")
.map(|path| {
let mut path = PathBuf::from(path);
path.push("Cargo.toml");
assert!(
path.exists(),
"Cargo manifest does not exist at path {}",
path.display()
);
path
})
.expect("CARGO_MANIFEST_DIR is not defined.")
}
fn get_manifest_modified_time(
cargo_manifest_path: &Path,
) -> Result<SystemTime, std::io::Error> {
std::fs::metadata(cargo_manifest_path).and_then(|metadata| metadata.modified())
}
fn read_manifest(path: &Path) -> DocumentMut {
let manifest = std::fs::read_to_string(path)
.unwrap_or_else(|_| panic!("Unable to read cargo manifest: {}", path.display()));
manifest
.parse::<DocumentMut>()
.unwrap_or_else(|_| panic!("Failed to parse cargo manifest: {}", path.display()))
}
/// Attempt to retrieve the [path](syn::Path) of a particular package in
/// the [manifest](BevyManifest) by [name](str).
pub fn maybe_get_path(&self, name: &str) -> Result<syn::Path, TryResolveCratePathError> {
self.0
.try_resolve_crate_path(name, ALL_KNOWN_RE_EXPORTING_CRATES)
}
pub fn maybe_get_path(&self, name: &str) -> Option<syn::Path> {
let find_in_deps = |deps: &Item| -> Option<syn::Path> {
let package = if deps.get(name).is_some() {
return Some(Self::parse_str(name));
} else if deps.get(BEVY).is_some() {
BEVY
} else {
// Note: to support bevy crate aliases, we could do scanning here to find a crate with a "package" name that
// matches our request, but that would then mean we are scanning every dependency (and dev dependency) for every
// macro execution that hits this branch (which includes all built-in bevy crates). Our current stance is that supporting
// remapped crate names in derive macros is not worth that "compile time" price of admission. As a workaround, people aliasing
// bevy crate names can use "use REMAPPED as bevy_X" or "use REMAPPED::x as bevy_x".
return None;
};
/// Returns the path for the crate with the given name.
pub fn get_path(&self, name: &str) -> syn::Path {
self.maybe_get_path(name)
.expect("Failed to get path for crate")
let mut path = Self::parse_str::<syn::Path>(package);
if let Some(module) = name.strip_prefix("bevy_") {
path.segments.push(Self::parse_str(module));
}
Some(path)
};
let deps = self.manifest.get("dependencies");
let deps_dev = self.manifest.get("dev-dependencies");
deps.and_then(find_in_deps)
.or_else(|| deps_dev.and_then(find_in_deps))
}
/// Attempt to parse the provided [path](str) as a [syntax tree node](syn::parse::Parse)
@ -56,6 +113,12 @@ impl BevyManifest {
syn::parse(path.parse::<TokenStream>().ok()?).ok()
}
/// Returns the path for the crate with the given name.
pub fn get_path(&self, name: &str) -> syn::Path {
self.maybe_get_path(name)
.unwrap_or_else(|| Self::parse_str(name))
}
/// Attempt to parse provided [path](str) as a [syntax tree node](syn::parse::Parse).
///
/// # Panics
@ -66,18 +129,4 @@ impl BevyManifest {
pub fn parse_str<T: syn::parse::Parse>(path: &str) -> T {
Self::try_parse_str(path).unwrap()
}
/// Attempt to get a subcrate [path](syn::Path) under Bevy by [name](str)
pub fn get_subcrate(&self, subcrate: &str) -> Result<syn::Path, TryResolveCratePathError> {
self.maybe_get_path(BEVY)
.map(|bevy_path| {
let mut segments = bevy_path.segments;
segments.push(BevyManifest::parse_str(subcrate));
syn::Path {
leading_colon: None,
segments,
}
})
.or_else(|_err| self.maybe_get_path(&format!("bevy_{subcrate}")))
}
}

1
crates/bevy_macro_utils/src/lib.rs
View File

@ -7,6 +7,7 @@
//! A collection of helper types and functions for working on macros within the Bevy ecosystem.
extern crate alloc;
extern crate proc_macro;
mod attrs;

6
tests-integration/remapped-test/Cargo.toml
View File

@ -1,6 +0,0 @@
[package]
name = "remapped-test"
edition = "2024"
[dependencies]
bevi = { path = "../../", package = "bevy" }

21
tests-integration/remapped-test/src/lib.rs
View File

@ -1,21 +0,0 @@
#![allow(dead_code)]
use bevi::prelude::*;
#[derive(Component)]
struct _Component {
_value: f32,
}
#[derive(Resource)]
struct _Resource {
_value: f32,
}
fn hello_world() {
println!("hello world!");
}
#[test]
fn simple_ecs_test() {
App::new().add_systems(Update, hello_world).run();
}