Skip to content

Commit dcd175f

Browse files
janezhang10meta-codesync[bot]
authored andcommitted
Add co_applyToVirtualInode to VirtualInodeLoader
Summary: Add a coroutine-native co_applyToVirtualInode that wraps the existing VirtualInodeLoader to resolve inodes, then applies the user-provided func via co_await instead of deferValue/collectAll. This is an intermediate step toward fully eliminating futures from the getBlake3 coroutine path — the loader itself still uses promises internally, but callers no longer need .semi() bridges on the outer ImmediateFuture. Also adds CO_TEST(CoInodeLoader, load) which mirrors the existing InodeLoader.load test, covering happy path, non-existent paths, duplicate paths, and invalid paths through the coroutine interface. Reviewed By: SBones Differential Revision: D101696722 fbshipit-source-id: a4455f880f17edaf36947d83e45d8e14d8fdda51
1 parent 0c20979 commit dcd175f

4 files changed

Lines changed: 158 additions & 1 deletion

File tree

eden/fs/inodes/BUCK

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,6 @@ cpp_library(
184184
"//folly:string",
185185
"//folly:unit",
186186
"//folly/chrono:conv",
187-
"//folly/coro:collect",
188187
"//folly/coro:invoke",
189188
"//folly/io:iobuf",
190189
"//folly/io/async:async_base",
@@ -263,6 +262,7 @@ cpp_library(
263262
"//folly/concurrency/memory:atomic_read_mostly_main_ptr",
264263
"//folly/concurrency/memory:read_mostly_shared_ptr",
265264
"//folly/container:evicting_cache_map",
265+
"//folly/coro:collect",
266266
"//folly/coro/safe:now_task",
267267
"//folly/functional:invoke",
268268
"//folly/futures:core",

eden/fs/inodes/VirtualInodeLoader.h

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
#include <folly/ExceptionWrapper.h>
1010
#include <folly/FBVector.h>
1111
#include <folly/MapUtil.h>
12+
#include <folly/coro/Collect.h>
13+
#include <folly/coro/safe/NowTask.h>
1214
#include <folly/functional/Invoke.h>
1315
#include <folly/futures/Future.h>
1416

@@ -190,4 +192,82 @@ auto applyToVirtualInode(
190192
});
191193
}
192194

195+
/**
196+
* Coroutine-native version of applyToVirtualInode. Uses the same
197+
* VirtualInodeLoader for efficient tree-shaped path resolution, but applies
198+
* func via co_await instead of deferValue/collectAll. func may return a
199+
* SemiFuture<T> or now_task<T> — anything co_awaitable.
200+
*
201+
* Both inode resolution and func application are parallelized: resolution
202+
* via the loader's tree-shaped plan, func via collectAllRange.
203+
*/
204+
template <typename Func>
205+
folly::coro::now_task<
206+
std::vector<folly::Try<typename folly::isFutureOrSemiFuture<
207+
folly::invoke_result_t<Func&, VirtualInode, RelativePath>>::Inner>>>
208+
co_applyToVirtualInode(
209+
InodePtr rootInode,
210+
const std::vector<std::string>& paths,
211+
Func func,
212+
const std::shared_ptr<ObjectStore>& store,
213+
const ObjectFetchContextPtr& fetchContext) {
214+
using FuncRet = folly::invoke_result_t<Func&, VirtualInode, RelativePath>;
215+
using Result = typename folly::isFutureOrSemiFuture<FuncRet>::Inner;
216+
217+
detail::VirtualInodeLoader loader;
218+
219+
// Func may not be copyable, so wrap it in a shared_ptr.
220+
auto cb = std::make_shared<Func>(std::move(func));
221+
222+
// Set up load futures for each path. If path parsing fails, store a failed
223+
// SemiFuture so the error appears in the corresponding result entry.
224+
std::vector<folly::SemiFuture<VirtualInode>> loadFutures;
225+
std::vector<RelativePath> relPaths;
226+
loadFutures.reserve(paths.size());
227+
relPaths.reserve(paths.size());
228+
for (const auto& path : paths) {
229+
try {
230+
auto relPath = RelativePathPiece{path};
231+
loadFutures.push_back(loader.load(relPath));
232+
relPaths.push_back(relPath.copy());
233+
} catch (const std::exception&) {
234+
loadFutures.push_back(
235+
folly::makeSemiFuture<VirtualInode>(
236+
folly::exception_wrapper(std::current_exception())));
237+
relPaths.emplace_back();
238+
}
239+
}
240+
241+
// Resolve all inodes via the loader's tree-shaped plan.
242+
co_await loader
243+
.loaded(
244+
folly::Try<VirtualInode>(VirtualInode{std::move(rootInode)}),
245+
RelativePath(),
246+
store,
247+
fetchContext)
248+
.semi();
249+
250+
// Apply func to each resolved inode in parallel.
251+
std::vector<folly::coro::Task<folly::Try<Result>>> tasks;
252+
tasks.reserve(loadFutures.size());
253+
for (size_t i = 0; i < loadFutures.size(); ++i) {
254+
tasks.push_back(
255+
folly::coro::co_invoke(
256+
[cb,
257+
loadFuture = std::move(loadFutures[i]),
258+
path = std::move(relPaths[i])]() mutable
259+
-> folly::coro::Task<folly::Try<Result>> {
260+
auto inodeTry =
261+
co_await folly::coro::co_awaitTry(std::move(loadFuture));
262+
if (inodeTry.hasException()) {
263+
co_return folly::Try<Result>(inodeTry.exception());
264+
}
265+
co_return co_await folly::coro::co_awaitTry(
266+
(*cb)(std::move(inodeTry.value()), std::move(path)));
267+
}));
268+
}
269+
270+
co_return co_await folly::coro::collectAllRange(std::move(tasks));
271+
}
272+
193273
} // namespace facebook::eden

eden/fs/inodes/test/BUCK

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,7 @@ cpp_unittest(
234234
"//eden/fs/testharness:fake_backing_store_and_tree_builder",
235235
"//eden/fs/testharness:test_checks",
236236
"//eden/fs/testharness:test_mount",
237+
"//folly/coro:gtest_helpers",
237238
"//folly/test:test_utils",
238239
"//folly/testing:test_util",
239240
],

eden/fs/inodes/test/VirtualInodeLoaderTest.cpp

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
*/
77

88
#include "eden/fs/inodes/VirtualInodeLoader.h"
9+
#include <folly/coro/GtestHelpers.h>
910
#include <folly/test/TestUtils.h>
1011
#include <folly/testing/TestUtil.h>
1112
#include <gtest/gtest.h>
@@ -132,3 +133,78 @@ TEST(InodeLoader, notReady) {
132133
EXPECT_EQ(Hash20::sha1("dir/sub/b.txt"), results[3].value());
133134
}
134135
}
136+
137+
CO_TEST(CoInodeLoader, load) {
138+
FakeTreeBuilder builder;
139+
builder.setFiles(FILES);
140+
TestMount mount(builder);
141+
142+
auto rootInode = mount.getTreeInode(RelativePathPiece());
143+
auto objectStore = mount.getEdenMount()->getObjectStore();
144+
auto fetchContext = ObjectFetchContext::getNullContext();
145+
146+
{
147+
auto results = co_await co_applyToVirtualInode(
148+
rootInode,
149+
std::vector<std::string>{
150+
"dir/a.txt", "not/exist/a", "not/exist/b", "dir/sub/b.txt"},
151+
[&](const VirtualInode& inode,
152+
const RelativePath& path) -> folly::SemiFuture<Hash32> {
153+
return inode.getBlake3(path, objectStore, fetchContext).semi();
154+
},
155+
objectStore,
156+
fetchContext);
157+
158+
EXPECT_EQ(
159+
Hash32::blake3(folly::ByteRange{folly::StringPiece{"dir/a.txt"}}),
160+
results[0].value());
161+
EXPECT_THROW_ERRNO(results[1].value(), ENOENT);
162+
EXPECT_THROW_ERRNO(results[2].value(), ENOENT);
163+
EXPECT_EQ(
164+
Hash32::blake3(folly::ByteRange{folly::StringPiece{"dir/sub/b.txt"}}),
165+
results[3].value());
166+
}
167+
168+
{
169+
auto results = co_await co_applyToVirtualInode(
170+
rootInode,
171+
std::vector<std::string>{
172+
"dir/sub/b.txt",
173+
"dir/a.txt",
174+
"not/exist/a",
175+
"not/exist/b",
176+
"dir/sub/b.txt"},
177+
[&](const VirtualInode& inode, const RelativePath& path) {
178+
return inode.getBlake3(path, objectStore, fetchContext).semi();
179+
},
180+
objectStore,
181+
fetchContext);
182+
183+
EXPECT_EQ(
184+
Hash32::blake3(folly::ByteRange{folly::StringPiece{"dir/sub/b.txt"}}),
185+
results[0].value());
186+
EXPECT_EQ(
187+
Hash32::blake3(folly::ByteRange{folly::StringPiece{"dir/a.txt"}}),
188+
results[1].value());
189+
EXPECT_THROW_ERRNO(results[2].value(), ENOENT);
190+
EXPECT_THROW_ERRNO(results[3].value(), ENOENT);
191+
EXPECT_EQ(results[0].value(), results[4].value())
192+
<< "dir/sub/b.txt was requested twice and both entries are the same";
193+
}
194+
195+
{
196+
auto results = co_await co_applyToVirtualInode(
197+
rootInode,
198+
std::vector<std::string>{"dir/a.txt", "/invalid///exist/a"},
199+
[&](const VirtualInode& inode, const RelativePath& path) {
200+
return inode.getBlake3(path, objectStore, fetchContext).semi();
201+
},
202+
objectStore,
203+
fetchContext);
204+
205+
EXPECT_EQ(
206+
Hash32::blake3(folly::ByteRange{folly::StringPiece{"dir/a.txt"}}),
207+
results[0].value());
208+
EXPECT_THROW_RE(results[1].value(), std::domain_error, "absolute path");
209+
}
210+
}

0 commit comments

Comments
 (0)