Skip to content

Commit bda2b29

Browse files
authored
feat: improve tracing spans, add profile_with_tracy feature flag (#394)
# Summary Overhauls tracing spans allowing neat visualisations like this one: ![image](https://github.com/user-attachments/assets/fcc62e04-7b6e-46e0-9a32-eb14dc4a1459) At the same time, expands benchmarks to cover ground on "marshalling" teritory, which seems to be a large bottleneck in performance. Also adds utilities to xtask to allow seamless tracing of BMS benchmarks with `tracy`
1 parent b4a5c90 commit bda2b29

File tree

28 files changed

+369
-75
lines changed

28 files changed

+369
-75
lines changed

.github/workflows/bencher_on_pr_or_fork_closed.yml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,5 @@ jobs:
1414
bencher archive \
1515
--project bms \
1616
--token '${{ secrets.BENCHER_API_TOKEN }}' \
17-
--branch "$GITHUB_HEAD_REF"
17+
--branch "$GITHUB_HEAD_REF"
18+
continue-on-error: true

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ script_integration_test_harness = { workspace = true }
8989
test_utils = { workspace = true }
9090
libtest-mimic = "0.8"
9191
tracing-tracy = "0.11"
92+
regex = "1.11"
9293

9394
[workspace]
9495
members = [

benches/benchmarks.rs

Lines changed: 154 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,19 @@
1-
use bevy::log::tracing_subscriber;
21
use bevy::log::tracing_subscriber::layer::SubscriberExt;
3-
use bevy::utils::{tracing, HashMap};
2+
use bevy::log::{tracing_subscriber, Level};
3+
use bevy::reflect::Reflect;
4+
use bevy::utils::tracing;
5+
use bevy::utils::tracing::span;
6+
use bevy_mod_scripting_core::bindings::{
7+
FromScript, IntoScript, Mut, Ref, ReflectReference, ScriptValue, Val,
8+
};
49
use criterion::{criterion_main, measurement::Measurement, BenchmarkGroup, Criterion};
5-
use script_integration_test_harness::{run_lua_benchmark, run_rhai_benchmark};
10+
use criterion::{BatchSize, BenchmarkFilter};
11+
use regex::Regex;
12+
use script_integration_test_harness::test_functions::rand::Rng;
13+
use script_integration_test_harness::{
14+
perform_benchmark_with_generator, run_lua_benchmark, run_rhai_benchmark,
15+
};
16+
use std::collections::HashMap;
617
use std::{path::PathBuf, sync::LazyLock, time::Duration};
718
use test_utils::{discover_all_tests, Test};
819

@@ -65,10 +76,24 @@ impl BenchmarkExecutor for Test {
6576
}
6677
}
6778

68-
fn script_benchmarks(criterion: &mut Criterion) {
79+
fn script_benchmarks(criterion: &mut Criterion, filter: Option<Regex>) {
6980
// find manifest dir
7081
let manifest_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
71-
let tests = discover_all_tests(manifest_dir, |p| p.starts_with("benchmarks"));
82+
let tests = discover_all_tests(manifest_dir, |p| {
83+
p.path.starts_with("benchmarks")
84+
&& if let Some(filter) = &filter {
85+
let matching = filter.is_match(&p.benchmark_name());
86+
if !matching {
87+
println!(
88+
"Skipping benchmark: '{}'. due to filter: '{filter}'",
89+
p.benchmark_name()
90+
);
91+
};
92+
matching
93+
} else {
94+
true
95+
}
96+
});
7297

7398
// group by benchmark group
7499
let mut grouped: HashMap<String, Vec<Test>> =
@@ -83,9 +108,16 @@ fn script_benchmarks(criterion: &mut Criterion) {
83108
}
84109

85110
for (group, tests) in grouped {
111+
println!("Running benchmarks for group: {}", group);
86112
let mut benchmark_group = criterion.benchmark_group(group);
87113

88114
for t in tests {
115+
println!("Running benchmark: {}", t.benchmark_name());
116+
span!(
117+
Level::INFO,
118+
"Benchmark harness for test",
119+
test_name = &t.benchmark_name()
120+
);
89121
t.execute(&mut benchmark_group);
90122
}
91123

@@ -104,22 +136,135 @@ fn maybe_with_profiler(f: impl Fn(bool)) {
104136

105137
tracing::subscriber::set_global_default(subscriber).unwrap();
106138

107-
let _ = tracing_tracy::client::span!("test2");
108-
tracing::info_span!("test");
109-
110139
f(true);
111140
} else {
112141
f(false);
113142
}
114143
}
115144

145+
/// benchmarks measuring conversion time for script values and other things
146+
fn conversion_benchmarks(criterion: &mut Criterion) {
147+
let mut group = criterion.benchmark_group("conversions");
148+
149+
#[derive(Reflect)]
150+
struct ReflectyVal(pub u32);
151+
152+
perform_benchmark_with_generator(
153+
"ScriptValue::List",
154+
&|rng, _| {
155+
let mut array = Vec::new();
156+
for _ in 0..10 {
157+
array.push(ScriptValue::Integer(rng.random()));
158+
}
159+
ScriptValue::List(array)
160+
},
161+
&|w, i| {
162+
let i = i.into_script(w.clone()).unwrap();
163+
let _ = Vec::<ScriptValue>::from_script(i, w).unwrap();
164+
},
165+
&mut group,
166+
BatchSize::SmallInput,
167+
);
168+
169+
perform_benchmark_with_generator(
170+
"ScriptValue::Map",
171+
&|rng, _| {
172+
let mut map = HashMap::default();
173+
for _ in 0..10 {
174+
map.insert(
175+
rng.random::<u32>().to_string(),
176+
ScriptValue::Integer(rng.random()),
177+
);
178+
}
179+
ScriptValue::Map(map)
180+
},
181+
&|w, i| {
182+
let i = i.into_script(w.clone()).unwrap();
183+
let _ = HashMap::<String, ScriptValue>::from_script(i, w).unwrap();
184+
},
185+
&mut group,
186+
BatchSize::SmallInput,
187+
);
188+
189+
perform_benchmark_with_generator(
190+
"ScriptValue::Reference::from_into",
191+
&|rng, world| {
192+
let allocator = world.allocator();
193+
let mut allocator = allocator.write();
194+
ReflectReference::new_allocated(ReflectyVal(rng.random()), &mut allocator)
195+
},
196+
&|w, i| {
197+
let i = i.into_script(w.clone()).unwrap();
198+
let _ = ReflectReference::from_script(i, w).unwrap();
199+
},
200+
&mut group,
201+
BatchSize::SmallInput,
202+
);
203+
204+
perform_benchmark_with_generator(
205+
"Val<T>::from_into",
206+
&|rng, _| Val::new(ReflectyVal(rng.random::<u32>())),
207+
&|w, i| {
208+
let v = i.into_script(w.clone()).unwrap();
209+
Val::<ReflectyVal>::from_script(v, w).unwrap();
210+
},
211+
&mut group,
212+
BatchSize::SmallInput,
213+
);
214+
215+
perform_benchmark_with_generator(
216+
"Ref<T>::from",
217+
&|rng, w| {
218+
Val::new(ReflectyVal(rng.random::<u32>()))
219+
.into_script(w)
220+
.unwrap()
221+
},
222+
&|w, i| {
223+
Ref::<ReflectyVal>::from_script(i, w).unwrap();
224+
},
225+
&mut group,
226+
BatchSize::SmallInput,
227+
);
228+
229+
perform_benchmark_with_generator(
230+
"Mut<T>::from",
231+
&|rng, w| {
232+
Val::new(ReflectyVal(rng.random::<u32>()))
233+
.into_script(w)
234+
.unwrap()
235+
},
236+
&|w, i| {
237+
Mut::<ReflectyVal>::from_script(i, w).unwrap();
238+
},
239+
&mut group,
240+
BatchSize::SmallInput,
241+
);
242+
}
243+
116244
pub fn benches() {
117245
maybe_with_profiler(|_profiler| {
118246
let mut criterion: criterion::Criterion<_> = (criterion::Criterion::default())
119247
.configure_from_args()
120248
.measurement_time(Duration::from_secs(10));
249+
let arguments = std::env::args()
250+
.skip(1) // the executable name
251+
.filter(|a| !a.starts_with("-"))
252+
.collect::<Vec<String>>();
253+
254+
// take first argument as .*<val>.* regex for benchmarks
255+
// criterion will already have that as a filter, but we want to make sure we're on the same page
256+
let filter = if let Some(n) = arguments.first() {
257+
println!("using filter: '{n}'");
258+
let regex = Regex::new(n).unwrap();
259+
let filter = BenchmarkFilter::Regex(regex.clone());
260+
criterion = criterion.with_benchmark_filter(filter);
261+
Some(regex)
262+
} else {
263+
None
264+
};
121265

122-
script_benchmarks(&mut criterion);
266+
script_benchmarks(&mut criterion, filter);
267+
conversion_benchmarks(&mut criterion);
123268
});
124269
}
125270
criterion_main!(benches);

crates/bevy_mod_scripting_core/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ bevy = { workspace = true, default-features = false, features = ["bevy_asset"] }
3333
thiserror = "1.0.31"
3434
parking_lot = "0.12.1"
3535
dashmap = "6"
36-
smallvec = "1.11"
36+
smallvec = { version = "1.11", features = ["union"] }
3737
itertools = "0.13"
3838
derivative = "2.2"
3939
profiling = { workspace = true }

crates/bevy_mod_scripting_core/src/asset.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ pub struct ScriptAssetLoader {
7373
pub preprocessor: Option<Box<dyn Fn(&mut [u8]) -> Result<(), ScriptError> + Send + Sync>>,
7474
}
7575

76+
#[profiling::all_functions]
7677
impl AssetLoader for ScriptAssetLoader {
7778
type Asset = ScriptAsset;
7879

@@ -121,6 +122,7 @@ pub struct ScriptAssetSettings {
121122
pub supported_extensions: &'static [&'static str],
122123
}
123124

125+
#[profiling::all_functions]
124126
impl ScriptAssetSettings {
125127
/// Selects the language for a given asset path
126128
pub fn select_script_language(&self, path: &AssetPath) -> Language {
@@ -178,6 +180,7 @@ pub struct ScriptMetadata {
178180
pub language: Language,
179181
}
180182

183+
#[profiling::all_functions]
181184
impl ScriptMetadataStore {
182185
/// Inserts a new metadata entry
183186
pub fn insert(&mut self, id: AssetId<ScriptAsset>, meta: ScriptMetadata) {
@@ -202,6 +205,7 @@ impl ScriptMetadataStore {
202205
}
203206

204207
/// Converts incoming asset events, into internal script asset events, also loads and inserts metadata for newly added scripts
208+
#[profiling::function]
205209
pub(crate) fn dispatch_script_asset_events(
206210
mut events: EventReader<AssetEvent<ScriptAsset>>,
207211
mut script_asset_events: EventWriter<ScriptAssetEvent>,
@@ -256,6 +260,7 @@ pub(crate) fn dispatch_script_asset_events(
256260
}
257261

258262
/// Listens to [`ScriptAssetEvent::Removed`] events and removes the corresponding script metadata.
263+
#[profiling::function]
259264
pub(crate) fn remove_script_metadata(
260265
mut events: EventReader<ScriptAssetEvent>,
261266
mut asset_path_map: ResMut<ScriptMetadataStore>,
@@ -273,6 +278,7 @@ pub(crate) fn remove_script_metadata(
273278
/// Listens to [`ScriptAssetEvent`] events and dispatches [`CreateOrUpdateScript`] and [`DeleteScript`] commands accordingly.
274279
///
275280
/// Allows for hot-reloading of scripts.
281+
#[profiling::function]
276282
pub(crate) fn sync_script_data<P: IntoScriptPluginParams>(
277283
mut events: EventReader<ScriptAssetEvent>,
278284
script_assets: Res<Assets<ScriptAsset>>,
@@ -321,6 +327,7 @@ pub(crate) fn sync_script_data<P: IntoScriptPluginParams>(
321327
}
322328

323329
/// Setup all the asset systems for the scripting plugin and the dependencies
330+
#[profiling::function]
324331
pub(crate) fn configure_asset_systems(app: &mut App) -> &mut App {
325332
// these should be in the same set as bevy's asset systems
326333
// currently this is in the PreUpdate set
@@ -348,6 +355,7 @@ pub(crate) fn configure_asset_systems(app: &mut App) -> &mut App {
348355
}
349356

350357
/// Setup all the asset systems for the scripting plugin and the dependencies
358+
#[profiling::function]
351359
pub(crate) fn configure_asset_systems_for_plugin<P: IntoScriptPluginParams>(
352360
app: &mut App,
353361
) -> &mut App {

0 commit comments

Comments
 (0)