Skip to content

Commit 5a4846e

Browse files
BromeonBromeon
authored
Merge pull request godot-rust#1167 from godot-rust/perf/boxed-call-error
Decrease `CallError` size from 176 to 8 bytes
2 parents 4d2251b + 554ada3 commit 5a4846e

File tree

2 files changed

+68
-63
lines changed

2 files changed

+68
-63
lines changed

‎godot-core/src/meta/error/call_error.rs

Lines changed: 42 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,14 @@ use std::fmt;
7171
/// assert_eq!(inner.class_name(), Some("MyClass"));
7272
/// assert_eq!(inner.method_name(), "my_method");
7373
/// }
74-
#[derive(Debug)]
7574
pub struct CallError {
75+
// Boxed since the original struct is >= 176 bytes, making Result<..., CallError> very large.
76+
b: Box<InnerCallError>,
77+
}
78+
79+
/// Inner struct. All functionality on outer `impl`.
80+
#[derive(Debug)]
81+
struct InnerCallError {
7682
class_name: String,
7783
function_name: String,
7884
call_expr: String,
@@ -92,16 +98,16 @@ impl CallError {
9298
/// This is the static and not the dynamic type. For example, if you invoke `call()` on a `Gd<Node>`, you are effectively invoking
9399
/// `Object::call()` (through `DerefMut`), and the class name will be `Object`.
94100
pub fn class_name(&self) -> Option<&str> {
95-
if self.class_name.is_empty() {
101+
if self.b.class_name.is_empty() {
96102
None
97103
} else {
98-
Some(&self.class_name)
104+
Some(&self.b.class_name)
99105
}
100106
}
101107

102108
/// Name of the function or method that failed.
103109
pub fn method_name(&self) -> &str {
104-
&self.function_name
110+
&self.b.function_name
105111
}
106112

107113
// ------------------------------------------------------------------------------------------------------------------------------------------
@@ -150,8 +156,7 @@ impl CallError {
150156
// If the call error encodes an error generated by us, decode it.
151157
let mut source_error = None;
152158
if err.error == sys::GODOT_RUST_CUSTOM_CALL_ERROR {
153-
source_error = crate::private::call_error_remove(&err) //.
154-
.map(|e| SourceError::Call(Box::new(e)));
159+
source_error = crate::private::call_error_remove(&err).map(SourceError::Call);
155160
}
156161

157162
Err(Self::failed_varcall_inner(
@@ -295,8 +300,8 @@ impl CallError {
295300
// source,
296301
// }
297302

298-
call_error.source = source;
299-
call_error.call_expr = call_expr;
303+
call_error.b.source = source;
304+
call_error.b.call_expr = call_expr;
300305
call_error
301306
}
302307

@@ -316,7 +321,7 @@ impl CallError {
316321
reason: impl Into<String>,
317322
source: Option<ConvertError>,
318323
) -> Self {
319-
Self {
324+
let inner = InnerCallError {
320325
class_name: call_ctx.class_name.to_string(),
321326
function_name: call_ctx.function_name.to_string(),
322327
call_expr: format!("{call_ctx}()"),
@@ -325,17 +330,22 @@ impl CallError {
325330
value: e.value().map_or_else(String::new, |v| format!("{:?}", v)),
326331
erased_error: e.into(),
327332
}),
328-
}
333+
};
334+
335+
Self { b: Box::new(inner) }
329336
}
330337

331338
/// Describes the error.
332339
///
333340
/// This is the same as the `Display`/`ToString` repr, but without the prefix mentioning that this is a function call error,
334341
/// and without any source error information.
335-
fn message(&self, with_source: bool) -> String {
336-
let Self {
337-
call_expr, reason, ..
338-
} = self;
342+
pub fn message(&self, with_source: bool) -> String {
343+
let InnerCallError {
344+
call_expr,
345+
reason,
346+
source,
347+
..
348+
} = &*self.b;
339349

340350
let reason_str = if reason.is_empty() {
341351
String::new()
@@ -351,7 +361,7 @@ impl CallError {
351361
// String::new()
352362
// };
353363

354-
let source_str = match &self.source {
364+
let source_str = match source {
355365
Some(SourceError::Convert {
356366
erased_error,
357367
value,
@@ -361,7 +371,10 @@ impl CallError {
361371
if value.is_empty() { "" } else { ": " },
362372
)
363373
}
364-
Some(SourceError::Call(e)) if with_source => format!("\n Source: {}", e.message(true)),
374+
Some(SourceError::Call(e)) if with_source => {
375+
let message = e.message(true);
376+
format!("\n Source: {message}")
377+
}
365378
_ => String::new(),
366379
};
367380

@@ -371,13 +384,21 @@ impl CallError {
371384

372385
impl fmt::Display for CallError {
373386
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
374-
write!(f, "godot-rust function call failed: {}", self.message(true))
387+
let message = self.message(true);
388+
write!(f, "godot-rust function call failed: {message}")
389+
}
390+
}
391+
392+
impl fmt::Debug for CallError {
393+
// Delegate to inner box.
394+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
395+
write!(f, "{:?}", self.b)
375396
}
376397
}
377398

378399
impl Error for CallError {
379400
fn source(&self) -> Option<&(dyn Error + 'static)> {
380-
match self.source.as_ref() {
401+
match self.b.source.as_ref() {
381402
Some(SourceError::Convert {
382403
erased_error: e, ..
383404
}) => deref_to::<ErasedConvertError>(e),
@@ -396,7 +417,9 @@ enum SourceError {
396417
erased_error: ErasedConvertError,
397418
value: String,
398419
},
399-
Call(Box<CallError>),
420+
421+
// If the top-level Box on CallError is ever removed, this would need to store Box<CallError> again.
422+
Call(CallError),
400423
}
401424

402425
/// Explicit dereferencing to a certain type. Avoids accidentally returning `&Box<T>` or so.

‎godot-core/src/tools/gfile.rs

Lines changed: 26 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -106,13 +106,10 @@ impl GFile {
106106
arg_into_ref!(path);
107107

108108
let fa = FileAccess::open(path, flags).ok_or_else(|| {
109-
std::io::Error::new(
110-
ErrorKind::Other,
111-
format!(
112-
"can't open file {path} in mode {flags:?}; GodotError: {:?}",
113-
FileAccess::get_open_error()
114-
),
115-
)
109+
std::io::Error::other(format!(
110+
"can't open file {path} in mode {flags:?}; GodotError: {:?}",
111+
FileAccess::get_open_error()
112+
))
116113
})?;
117114

118115
Ok(Self::from_inner(fa))
@@ -133,13 +130,10 @@ impl GFile {
133130
.compression_mode(compression_mode)
134131
.done()
135132
.ok_or_else(|| {
136-
std::io::Error::new(
137-
ErrorKind::Other,
138-
format!(
139-
"can't open file {path} in mode {flags:?}; GodotError: {:?}",
140-
FileAccess::get_open_error()
141-
),
142-
)
133+
std::io::Error::other(format!(
134+
"can't open file {path} in mode {flags:?}; GodotError: {:?}",
135+
FileAccess::get_open_error()
136+
))
143137
})?;
144138

145139
Ok(Self::from_inner(fa))
@@ -157,13 +151,10 @@ impl GFile {
157151
arg_into_ref!(path);
158152

159153
let fa = FileAccess::open_encrypted(path, flags, key).ok_or_else(|| {
160-
std::io::Error::new(
161-
ErrorKind::Other,
162-
format!(
163-
"can't open file {path} in mode {flags:?}; GodotError: {:?}",
164-
FileAccess::get_open_error()
165-
),
166-
)
154+
std::io::Error::other(format!(
155+
"can't open file {path} in mode {flags:?}; GodotError: {:?}",
156+
FileAccess::get_open_error()
157+
))
167158
})?;
168159

169160
Ok(Self::from_inner(fa))
@@ -182,13 +173,10 @@ impl GFile {
182173
arg_into_ref!(password);
183174

184175
let fa = FileAccess::open_encrypted_with_pass(path, flags, password).ok_or_else(|| {
185-
std::io::Error::new(
186-
ErrorKind::Other,
187-
format!(
188-
"can't open file {path} in mode {flags:?}; GodotError: {:?}",
189-
FileAccess::get_open_error()
190-
),
191-
)
176+
std::io::Error::other(format!(
177+
"can't open file {path} in mode {flags:?}; GodotError: {:?}",
178+
FileAccess::get_open_error()
179+
))
192180
})?;
193181
Ok(Self::from_inner(fa))
194182
}
@@ -224,10 +212,9 @@ impl GFile {
224212
let modified_time = FileAccess::get_modified_time(path);
225213

226214
if modified_time == 0 {
227-
Err(std::io::Error::new(
228-
ErrorKind::Other,
229-
format!("can't retrieve last modified time: {path}"),
230-
))
215+
Err(std::io::Error::other(format!(
216+
"can't retrieve last modified time: {path}"
217+
)))
231218
} else {
232219
Ok(modified_time)
233220
}
@@ -240,10 +227,9 @@ impl GFile {
240227
let md5 = FileAccess::get_md5(path);
241228

242229
if md5.is_empty() {
243-
Err(std::io::Error::new(
244-
ErrorKind::Other,
245-
format!("failed to compute file's MD5 checksum: {path}"),
246-
))
230+
Err(std::io::Error::other(format!(
231+
"failed to compute file's MD5 checksum: {path}"
232+
)))
247233
} else {
248234
Ok(md5)
249235
}
@@ -256,10 +242,9 @@ impl GFile {
256242
let sha256 = FileAccess::get_sha256(path);
257243

258244
if sha256.is_empty() {
259-
Err(std::io::Error::new(
260-
ErrorKind::Other,
261-
format!("failed to compute file's SHA-256 checksum: {path}"),
262-
))
245+
Err(std::io::Error::other(format!(
246+
"failed to compute file's SHA-256 checksum: {path}"
247+
)))
263248
} else {
264249
Ok(sha256)
265250
}
@@ -684,10 +669,7 @@ impl GFile {
684669
return Ok(());
685670
}
686671

687-
Err(std::io::Error::new(
688-
ErrorKind::Other,
689-
format!("GodotError: {:?}", error),
690-
))
672+
Err(std::io::Error::other(format!("GodotError: {:?}", error)))
691673
}
692674

693675
// File length cache is stored and kept when possible because `FileAccess::get_length()` turned out to be slowing down

0 commit comments

Comments
 (0)