Skip to content

Commit ec292e2

Browse files
Chris DrouillardCQ Bot
Chris Drouillard
authored and
CQ Bot
committed
[fxfs] Reduce the size futures with guards
Rust futures generated from async move {} blocks are at least double the size of all captured variables: rust-lang/rust#108906 There are 3 locations in fxfs where a future and a guard object are captured inside of another future with the purpose of running the original future while the guard is held. Rolling our own future for this specific case halves the size of the outer futures. This reduces the size of the future spawned in FxFile::create_connection_async for prefetch_keys from 10032 bytes to 5024 bytes. Bug: b/393365596 Change-Id: Id46f9939191a5ec63076a4ad35d220a00831152f Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1198687 Commit-Queue: Chris Drouillard <[email protected]> Reviewed-by: James Sullivan <[email protected]>
1 parent 73c5a9c commit ec292e2

File tree

6 files changed

+114
-14
lines changed

6 files changed

+114
-14
lines changed

src/storage/fxfs/BUILD.gn

+2
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ fxfs_deps = [
4242
"//third_party/rust_crates:log",
4343
"//third_party/rust_crates:num-traits",
4444
"//third_party/rust_crates:once_cell",
45+
"//third_party/rust_crates:pin-project",
4546
"//third_party/rust_crates:rand",
4647
"//third_party/rust_crates:rustc-hash",
4748
"//third_party/rust_crates:scopeguard",
@@ -67,6 +68,7 @@ fxfs_sources = [
6768
"src/fsck/errors.rs",
6869
"src/fsck/store_scanner.rs",
6970
"src/fsck/tests.rs",
71+
"src/future_with_guard.rs",
7072
"src/lib.rs",
7173
"src/log.rs",
7274
"src/lsm_tree.rs",

src/storage/fxfs/platform/src/fuchsia/file.rs

+2-4
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use anyhow::Error;
1414
use fidl_fuchsia_io as fio;
1515
use futures::future::BoxFuture;
1616
use fxfs::filesystem::{SyncOptions, MAX_FILE_SIZE};
17+
use fxfs::future_with_guard::FutureWithGuard;
1718
use fxfs::log::*;
1819
use fxfs::object_handle::{ObjectHandle, ReadObjectHandle};
1920
use fxfs::object_store::transaction::{lock_keys, LockKey, Options};
@@ -240,10 +241,7 @@ impl FxFile {
240241
)))
241242
.await
242243
.into_owned(fs);
243-
this.handle.owner().scope().spawn(async move {
244-
let _read_lock = read_lock;
245-
fut.await
246-
});
244+
this.handle.owner().scope().spawn(FutureWithGuard::new(read_lock, fut));
247245
}
248246
}
249247
}

src/storage/fxfs/platform/src/fuchsia/pager.rs

+2-5
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use crate::fuchsia::profile::Recorder;
99
use anyhow::Error;
1010
use bitflags::bitflags;
1111
use fuchsia_async as fasync;
12+
use fxfs::future_with_guard::FutureWithGuard;
1213
use fxfs::log::*;
1314
use fxfs::range::RangeExt;
1415
use fxfs::round::{round_down, round_up};
@@ -188,11 +189,7 @@ impl Pager {
188189

189190
/// Spawns a short term task for the pager that includes a guard that will prevent termination.
190191
fn spawn(&self, task: impl Future<Output = ()> + Send + 'static) {
191-
let guard = self.scope.active_guard();
192-
self.executor.spawn_detached(async move {
193-
task.await;
194-
std::mem::drop(guard);
195-
});
192+
self.executor.spawn_detached(FutureWithGuard::new(self.scope.active_guard(), task));
196193
}
197194

198195
/// Set the current profile recorder, or set to None to not record.

src/storage/fxfs/platform/src/fuchsia/volume.rs

+2-5
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ use futures::stream::{self, FusedStream, Stream};
2626
use futures::{FutureExt, StreamExt, TryStreamExt};
2727
use fxfs::errors::FxfsError;
2828
use fxfs::filesystem::{self, SyncOptions};
29+
use fxfs::future_with_guard::FutureWithGuard;
2930
use fxfs::log::*;
3031
use fxfs::object_store::directory::Directory;
3132
use fxfs::object_store::transaction::{lock_keys, LockKey, Options};
@@ -527,11 +528,7 @@ impl FxVolume {
527528

528529
/// Spawns a short term task for the volume that includes a guard that will prevent termination.
529530
pub fn spawn(&self, task: impl Future<Output = ()> + Send + 'static) {
530-
let guard = self.scope().active_guard();
531-
self.executor.spawn_detached(async move {
532-
task.await;
533-
std::mem::drop(guard);
534-
});
531+
self.executor.spawn_detached(FutureWithGuard::new(self.scope.active_guard(), task));
535532
}
536533

537534
/// Tries to unwrap this volume. If it fails, it will poison the volume so that when it is
+105
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
// Copyright 2025 The Fuchsia Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
use pin_project::pin_project;
6+
use std::future::Future;
7+
use std::pin::Pin;
8+
use std::task::{Context, Poll};
9+
10+
/// A wrapper around a future and a guard object where the correctness of the future requires the
11+
/// guard object to be held while the future is alive.
12+
///
13+
/// This is equivalent to the below code but produces a future that is almost half the size of the
14+
/// future that rust generates: https://github.com/rust-lang/rust/issues/108906.
15+
/// ```rust
16+
/// let guard = acquire_guard();
17+
/// executor.spawn(async move {
18+
/// let _guard = guard;
19+
/// task.await;
20+
/// });
21+
/// ```
22+
#[pin_project]
23+
pub struct FutureWithGuard<T, F, R>
24+
where
25+
F: Future<Output = R> + Send + 'static,
26+
T: Send + 'static,
27+
{
28+
#[pin]
29+
future: F,
30+
_object: T,
31+
}
32+
33+
impl<T, F, R> FutureWithGuard<T, F, R>
34+
where
35+
F: Future<Output = R> + Send + 'static,
36+
T: Send + 'static,
37+
{
38+
pub fn new(object: T, future: F) -> Self {
39+
Self { future, _object: object }
40+
}
41+
}
42+
43+
impl<T, F, R> Future for FutureWithGuard<T, F, R>
44+
where
45+
F: Future<Output = R> + Send + 'static,
46+
T: Send + 'static,
47+
{
48+
type Output = R;
49+
50+
fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
51+
self.project().future.poll(cx)
52+
}
53+
}
54+
55+
#[cfg(test)]
56+
mod tests {
57+
use super::*;
58+
59+
#[fuchsia::test]
60+
fn test_custom_future_is_smaller() {
61+
async fn large_future() -> [u8; 128] {
62+
let held_across_await = [0; 128];
63+
std::future::pending::<()>().await;
64+
held_across_await
65+
}
66+
67+
let object = 10u64;
68+
let task = large_future();
69+
let custom_future = FutureWithGuard::new(object, task);
70+
let custom_future_size = std::mem::size_of_val(&custom_future);
71+
72+
let object = 10u64;
73+
let task = large_future();
74+
let rust_future = async move {
75+
let _object = object;
76+
task.await;
77+
};
78+
let rust_future_size = std::mem::size_of_val(&rust_future);
79+
80+
// The large_future is 129 bytes:
81+
// - 128 bytes for the array
82+
// - 1 byte for the discriminant.
83+
//
84+
// The custom_future is 144 bytes:
85+
// - 129 bytes for the large_future
86+
// - 8 bytes for the u64 object
87+
// - 7 bytes of padding
88+
//
89+
// The rust_future is 272 bytes:
90+
// - 129 bytes to capture the large_future
91+
// - 129 bytes to use the large_future
92+
// - 8 bytes to capture the u64 object
93+
// - 1 byte for the discriminant
94+
// - 5 bytes of padding
95+
//
96+
// This assert only makes sure that the custom future is not bigger than the rust generated
97+
// future.
98+
//
99+
// If this test starts failing while updating rust, the test can safely be disabled.
100+
assert!(
101+
custom_future_size <= rust_future_size,
102+
"custom_future_size={custom_future_size} rust_future_size={rust_future_size}"
103+
);
104+
}
105+
}

src/storage/fxfs/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ mod debug_assert_not_too_long;
1919
pub mod errors;
2020
pub mod filesystem;
2121
pub mod fsck;
22+
pub mod future_with_guard;
2223
pub mod log;
2324
pub mod lsm_tree;
2425
pub mod metrics;

0 commit comments

Comments
 (0)