Skip to content

Commit e713c7b

Browse files
authored
Change bevy_math to use inline instead of inline(always) (#20887)
## Objective Move `bevy_math` closer to recommended inlining practices, and avoid problems with debuggers and optimising for size. ## Background Some `bevy_math` modules apply `#[inline(always)]` to almost every function. This has downsides for some users - it can prevent optimising for size, and can stop the debugger from stepping into functions. I can't find any source that advises `inline(always)` by default - the most common advice is "use rarely and only after profiling" (example: [std lib guide](https://std-dev-guide.rust-lang.org/policy/inline.html)). I've poked around the `bevy_math` history but couldn't find anything that explains why `inline(always)` was chosen. ## Solution This PR changes all instances of `#[inline(always)]` to `#[inline]`. The change is very unlikely to make any difference in optimised builds - almost all the functions are tiny so they're going to be inlined either way. Benchmarks showed no difference. The change can sometimes decrease performance in `opt-level = 0` builds - one math heavy microbenchmark took a -10% hit. But this is arguably the right trade-off if it lets the user step into functions in the debugger. Overall, I think this is the safer default for most users. `inline(always)` has several concrete downsides, while `inline` has some trade-offs but no clear downsides. The change also adds a new `bevy_math` benchmark which includes some of the affected functions. ## Alternatives The change could have been taken further. - Remove `#[inline]` from heftier functions like `BoundingSphere::from_point_cloud`. - Could help optimising for size. I left this out to keep things simple. - Remove `#[inline]` entirely. - I think this is likely to be a good thing, but it needs more testing and would probably be controversial. ## Testing ```sh cargo bench -p benches --bench math ``` Also: - Checked benchmark disassembly to confirm what was inlined. - Compiled `alien_cake_addict` with various optimisation levels to check there weren't major differences in size. EDIT: More details in [comment 1](#20887 (comment)), [comment 2](#20887 (comment)).
1 parent 0fc6c66 commit e713c7b

File tree

10 files changed

+425
-354
lines changed

10 files changed

+425
-354
lines changed

benches/Cargo.toml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,10 @@ bevy_platform = { path = "../crates/bevy_platform", default-features = false, fe
3131
] }
3232

3333
# Other crates
34-
glam = { version = "0.30.7", default-features = false, features = ["std"] }
34+
glam = { version = "0.30.7", default-features = false, features = [
35+
"std",
36+
"rand",
37+
] }
3538
rand = "0.9"
3639
rand_chacha = "0.9"
3740
nonmax = { version = "0.5", default-features = false }
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
use benches::bench;
2+
use bevy_math::{
3+
bounding::{Aabb3d, BoundingSphere, BoundingVolume},
4+
prelude::*,
5+
};
6+
use core::hint::black_box;
7+
use criterion::{criterion_group, Criterion};
8+
use rand::{
9+
distr::{Distribution, StandardUniform, Uniform},
10+
rngs::StdRng,
11+
Rng, SeedableRng,
12+
};
13+
14+
criterion_group!(benches, bounding);
15+
16+
struct PointCloud {
17+
points: Vec<Vec3A>,
18+
isometry: Isometry3d,
19+
}
20+
21+
impl PointCloud {
22+
fn aabb(&self) -> Aabb3d {
23+
Aabb3d::from_point_cloud(self.isometry, self.points.iter().copied())
24+
}
25+
26+
fn sphere(&self) -> BoundingSphere {
27+
BoundingSphere::from_point_cloud(self.isometry, &self.points)
28+
}
29+
}
30+
31+
fn bounding(c: &mut Criterion) {
32+
// Create point clouds of various sizes, then benchmark two different ways
33+
// of finding the bounds of each cloud and merging them together.
34+
35+
let mut rng1 = StdRng::seed_from_u64(123);
36+
let mut rng2 = StdRng::seed_from_u64(456);
37+
38+
let point_clouds = Uniform::<usize>::new(black_box(3), black_box(30))
39+
.unwrap()
40+
.sample_iter(&mut rng1)
41+
.take(black_box(1000))
42+
.map(|num_points| PointCloud {
43+
points: StandardUniform
44+
.sample_iter(&mut rng2)
45+
.take(num_points)
46+
.collect::<Vec<Vec3A>>(),
47+
isometry: Isometry3d::new(rng2.random::<Vec3>(), rng2.random::<Quat>()),
48+
})
49+
.collect::<Vec<_>>();
50+
51+
c.bench_function(bench!("bounding"), |b| {
52+
b.iter(|| {
53+
let aabb = point_clouds
54+
.iter()
55+
.map(PointCloud::aabb)
56+
.reduce(|l, r| l.merge(&r));
57+
58+
let sphere = point_clouds
59+
.iter()
60+
.map(PointCloud::sphere)
61+
.reduce(|l, r| l.merge(&r));
62+
63+
black_box(aabb);
64+
black_box(sphere);
65+
});
66+
});
67+
}

benches/benches/bevy_math/main.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use criterion::criterion_main;
22

33
mod bezier;
4+
mod bounding;
45

5-
criterion_main!(bezier::benches);
6+
criterion_main!(bezier::benches, bounding::benches);

crates/bevy_math/src/bounding/bounded2d/mod.rs

Lines changed: 37 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use bevy_reflect::{ReflectDeserialize, ReflectSerialize};
1515
use serde::{Deserialize, Serialize};
1616

1717
/// Computes the geometric center of the given set of points.
18-
#[inline(always)]
18+
#[inline]
1919
fn point_cloud_2d_center(points: &[Vec2]) -> Vec2 {
2020
assert!(
2121
!points.is_empty(),
@@ -56,7 +56,7 @@ pub struct Aabb2d {
5656

5757
impl Aabb2d {
5858
/// Constructs an AABB from its center and half-size.
59-
#[inline(always)]
59+
#[inline]
6060
pub fn new(center: Vec2, half_size: Vec2) -> Self {
6161
debug_assert!(half_size.x >= 0.0 && half_size.y >= 0.0);
6262
Self {
@@ -71,7 +71,7 @@ impl Aabb2d {
7171
/// # Panics
7272
///
7373
/// Panics if the given set of points is empty.
74-
#[inline(always)]
74+
#[inline]
7575
pub fn from_point_cloud(isometry: impl Into<Isometry2d>, points: &[Vec2]) -> Aabb2d {
7676
let isometry = isometry.into();
7777

@@ -93,7 +93,7 @@ impl Aabb2d {
9393
}
9494

9595
/// Computes the smallest [`BoundingCircle`] containing this [`Aabb2d`].
96-
#[inline(always)]
96+
#[inline]
9797
pub fn bounding_circle(&self) -> BoundingCircle {
9898
let radius = self.min.distance(self.max) / 2.0;
9999
BoundingCircle::new(self.center(), radius)
@@ -103,7 +103,7 @@ impl Aabb2d {
103103
///
104104
/// If the point is outside the AABB, the returned point will be on the perimeter of the AABB.
105105
/// Otherwise, it will be inside the AABB and returned as is.
106-
#[inline(always)]
106+
#[inline]
107107
pub fn closest_point(&self, point: Vec2) -> Vec2 {
108108
// Clamp point coordinates to the AABB
109109
point.clamp(self.min, self.max)
@@ -115,39 +115,39 @@ impl BoundingVolume for Aabb2d {
115115
type Rotation = Rot2;
116116
type HalfSize = Vec2;
117117

118-
#[inline(always)]
118+
#[inline]
119119
fn center(&self) -> Self::Translation {
120120
(self.min + self.max) / 2.
121121
}
122122

123-
#[inline(always)]
123+
#[inline]
124124
fn half_size(&self) -> Self::HalfSize {
125125
(self.max - self.min) / 2.
126126
}
127127

128-
#[inline(always)]
128+
#[inline]
129129
fn visible_area(&self) -> f32 {
130130
let b = (self.max - self.min).max(Vec2::ZERO);
131131
b.x * b.y
132132
}
133133

134-
#[inline(always)]
134+
#[inline]
135135
fn contains(&self, other: &Self) -> bool {
136136
other.min.x >= self.min.x
137137
&& other.min.y >= self.min.y
138138
&& other.max.x <= self.max.x
139139
&& other.max.y <= self.max.y
140140
}
141141

142-
#[inline(always)]
142+
#[inline]
143143
fn merge(&self, other: &Self) -> Self {
144144
Self {
145145
min: self.min.min(other.min),
146146
max: self.max.max(other.max),
147147
}
148148
}
149149

150-
#[inline(always)]
150+
#[inline]
151151
fn grow(&self, amount: impl Into<Self::HalfSize>) -> Self {
152152
let amount = amount.into();
153153
let b = Self {
@@ -158,7 +158,7 @@ impl BoundingVolume for Aabb2d {
158158
b
159159
}
160160

161-
#[inline(always)]
161+
#[inline]
162162
fn shrink(&self, amount: impl Into<Self::HalfSize>) -> Self {
163163
let amount = amount.into();
164164
let b = Self {
@@ -169,7 +169,7 @@ impl BoundingVolume for Aabb2d {
169169
b
170170
}
171171

172-
#[inline(always)]
172+
#[inline]
173173
fn scale_around_center(&self, scale: impl Into<Self::HalfSize>) -> Self {
174174
let scale = scale.into();
175175
let b = Self {
@@ -187,7 +187,7 @@ impl BoundingVolume for Aabb2d {
187187
/// Note that the result may not be as tightly fitting as the original, and repeated rotations
188188
/// can cause the AABB to grow indefinitely. Avoid applying multiple rotations to the same AABB,
189189
/// and consider storing the original AABB and rotating that every time instead.
190-
#[inline(always)]
190+
#[inline]
191191
fn transformed_by(
192192
mut self,
193193
translation: impl Into<Self::Translation>,
@@ -204,7 +204,7 @@ impl BoundingVolume for Aabb2d {
204204
/// Note that the result may not be as tightly fitting as the original, and repeated rotations
205205
/// can cause the AABB to grow indefinitely. Avoid applying multiple rotations to the same AABB,
206206
/// and consider storing the original AABB and rotating that every time instead.
207-
#[inline(always)]
207+
#[inline]
208208
fn transform_by(
209209
&mut self,
210210
translation: impl Into<Self::Translation>,
@@ -214,7 +214,7 @@ impl BoundingVolume for Aabb2d {
214214
self.translate_by(translation);
215215
}
216216

217-
#[inline(always)]
217+
#[inline]
218218
fn translate_by(&mut self, translation: impl Into<Self::Translation>) {
219219
let translation = translation.into();
220220
self.min += translation;
@@ -228,7 +228,7 @@ impl BoundingVolume for Aabb2d {
228228
/// Note that the result may not be as tightly fitting as the original, and repeated rotations
229229
/// can cause the AABB to grow indefinitely. Avoid applying multiple rotations to the same AABB,
230230
/// and consider storing the original AABB and rotating that every time instead.
231-
#[inline(always)]
231+
#[inline]
232232
fn rotated_by(mut self, rotation: impl Into<Self::Rotation>) -> Self {
233233
self.rotate_by(rotation);
234234
self
@@ -241,7 +241,7 @@ impl BoundingVolume for Aabb2d {
241241
/// Note that the result may not be as tightly fitting as the original, and repeated rotations
242242
/// can cause the AABB to grow indefinitely. Avoid applying multiple rotations to the same AABB,
243243
/// and consider storing the original AABB and rotating that every time instead.
244-
#[inline(always)]
244+
#[inline]
245245
fn rotate_by(&mut self, rotation: impl Into<Self::Rotation>) {
246246
let rot_mat = Mat2::from(rotation.into());
247247
let half_size = rot_mat.abs() * self.half_size();
@@ -250,7 +250,7 @@ impl BoundingVolume for Aabb2d {
250250
}
251251

252252
impl IntersectsVolume<Self> for Aabb2d {
253-
#[inline(always)]
253+
#[inline]
254254
fn intersects(&self, other: &Self) -> bool {
255255
let x_overlaps = self.min.x <= other.max.x && self.max.x >= other.min.x;
256256
let y_overlaps = self.min.y <= other.max.y && self.max.y >= other.min.y;
@@ -259,7 +259,7 @@ impl IntersectsVolume<Self> for Aabb2d {
259259
}
260260

261261
impl IntersectsVolume<BoundingCircle> for Aabb2d {
262-
#[inline(always)]
262+
#[inline]
263263
fn intersects(&self, circle: &BoundingCircle) -> bool {
264264
let closest_point = self.closest_point(circle.center);
265265
let distance_squared = circle.center.distance_squared(closest_point);
@@ -492,7 +492,7 @@ pub struct BoundingCircle {
492492

493493
impl BoundingCircle {
494494
/// Constructs a bounding circle from its center and radius.
495-
#[inline(always)]
495+
#[inline]
496496
pub fn new(center: Vec2, radius: f32) -> Self {
497497
debug_assert!(radius >= 0.);
498498
Self {
@@ -505,7 +505,7 @@ impl BoundingCircle {
505505
/// transformed by the rotation and translation of the given isometry.
506506
///
507507
/// The bounding circle is not guaranteed to be the smallest possible.
508-
#[inline(always)]
508+
#[inline]
509509
pub fn from_point_cloud(isometry: impl Into<Isometry2d>, points: &[Vec2]) -> BoundingCircle {
510510
let isometry = isometry.into();
511511

@@ -524,13 +524,13 @@ impl BoundingCircle {
524524
}
525525

526526
/// Get the radius of the bounding circle
527-
#[inline(always)]
527+
#[inline]
528528
pub fn radius(&self) -> f32 {
529529
self.circle.radius
530530
}
531531

532532
/// Computes the smallest [`Aabb2d`] containing this [`BoundingCircle`].
533-
#[inline(always)]
533+
#[inline]
534534
pub fn aabb_2d(&self) -> Aabb2d {
535535
Aabb2d {
536536
min: self.center - Vec2::splat(self.radius()),
@@ -542,7 +542,7 @@ impl BoundingCircle {
542542
///
543543
/// If the point is outside the circle, the returned point will be on the perimeter of the circle.
544544
/// Otherwise, it will be inside the circle and returned as is.
545-
#[inline(always)]
545+
#[inline]
546546
pub fn closest_point(&self, point: Vec2) -> Vec2 {
547547
self.circle.closest_point(point - self.center) + self.center
548548
}
@@ -553,28 +553,28 @@ impl BoundingVolume for BoundingCircle {
553553
type Rotation = Rot2;
554554
type HalfSize = f32;
555555

556-
#[inline(always)]
556+
#[inline]
557557
fn center(&self) -> Self::Translation {
558558
self.center
559559
}
560560

561-
#[inline(always)]
561+
#[inline]
562562
fn half_size(&self) -> Self::HalfSize {
563563
self.radius()
564564
}
565565

566-
#[inline(always)]
566+
#[inline]
567567
fn visible_area(&self) -> f32 {
568568
core::f32::consts::PI * self.radius() * self.radius()
569569
}
570570

571-
#[inline(always)]
571+
#[inline]
572572
fn contains(&self, other: &Self) -> bool {
573573
let diff = self.radius() - other.radius();
574574
self.center.distance_squared(other.center) <= ops::copysign(diff.squared(), diff)
575575
}
576576

577-
#[inline(always)]
577+
#[inline]
578578
fn merge(&self, other: &Self) -> Self {
579579
let diff = other.center - self.center;
580580
let length = diff.length();
@@ -591,42 +591,42 @@ impl BoundingVolume for BoundingCircle {
591591
)
592592
}
593593

594-
#[inline(always)]
594+
#[inline]
595595
fn grow(&self, amount: impl Into<Self::HalfSize>) -> Self {
596596
let amount = amount.into();
597597
debug_assert!(amount >= 0.);
598598
Self::new(self.center, self.radius() + amount)
599599
}
600600

601-
#[inline(always)]
601+
#[inline]
602602
fn shrink(&self, amount: impl Into<Self::HalfSize>) -> Self {
603603
let amount = amount.into();
604604
debug_assert!(amount >= 0.);
605605
debug_assert!(self.radius() >= amount);
606606
Self::new(self.center, self.radius() - amount)
607607
}
608608

609-
#[inline(always)]
609+
#[inline]
610610
fn scale_around_center(&self, scale: impl Into<Self::HalfSize>) -> Self {
611611
let scale = scale.into();
612612
debug_assert!(scale >= 0.);
613613
Self::new(self.center, self.radius() * scale)
614614
}
615615

616-
#[inline(always)]
616+
#[inline]
617617
fn translate_by(&mut self, translation: impl Into<Self::Translation>) {
618618
self.center += translation.into();
619619
}
620620

621-
#[inline(always)]
621+
#[inline]
622622
fn rotate_by(&mut self, rotation: impl Into<Self::Rotation>) {
623623
let rotation: Rot2 = rotation.into();
624624
self.center = rotation * self.center;
625625
}
626626
}
627627

628628
impl IntersectsVolume<Self> for BoundingCircle {
629-
#[inline(always)]
629+
#[inline]
630630
fn intersects(&self, other: &Self) -> bool {
631631
let center_distance_squared = self.center.distance_squared(other.center);
632632
let radius_sum_squared = (self.radius() + other.radius()).squared();
@@ -635,7 +635,7 @@ impl IntersectsVolume<Self> for BoundingCircle {
635635
}
636636

637637
impl IntersectsVolume<Aabb2d> for BoundingCircle {
638-
#[inline(always)]
638+
#[inline]
639639
fn intersects(&self, aabb: &Aabb2d) -> bool {
640640
aabb.intersects(self)
641641
}

0 commit comments

Comments
 (0)