Skip to content

Commit

Permalink
Return by value instead of reference in ProcessInfoCache
Browse files Browse the repository at this point in the history
Summary:
In the next diff we no longer have a valid reference to return. Gonna change
this to return by value.

The reason is that we are no longer gonna be able to return the value directly
from a stored future. We need to have a seperate future that we can block on
for each wait call. This means creating a new future for each call. And then
returning the value from the newly created future. This means the future we
return the value from might get destroyed after the call to this method. Thus
the value will outlive the future it's from.

There are maybe some things we could do to prevent having to return by value
instead of reference. But all the ideas I come up with are quite
complicated. This call expects to be "slow" anyways since it blocks waiting
for the pid info to be ready. So one extra copy on this path is not bad.

There are a few callers of this. We can remve a copy from these and replace them
with moves now that the returned value is owned.

Callers:
- [no update needed] https://www.internalfb.com/code/fbsource/[cc7496c5eba48918a08d7ac091ae154e75f9470e]/fbcode/eden/common/utils/test/ProcessInfoCacheTest.cpp?lines=123%2C131-132%2C135%2C138%2C171%2C201
- [updated] https://www.internalfb.com/code/fbsource/[cc7496c5eba48918a08d7ac091ae154e75f9470e]/fbcode/eden/fs/service/EdenServer.cpp?lines=2517
- [updated] https://www.internalfb.com/code/fbsource/[cc7496c5eba48918a08d7ac091ae154e75f9470e]/fbcode/watchman/Client.cpp?lines=285
- [no update needed] https://www.internalfb.com/code/fbsource/[cc7496c5eba48918a08d7ac091ae154e75f9470e]/fbcode/watchman/main.cpp?lines=117

Reviewed By: MichaelCuevas

Differential Revision: D61478484

fbshipit-source-id: fa3f82efb328b37ec7e19355ea835881b74948c2
  • Loading branch information
Katie Mancini authored and facebook-github-bot committed Aug 22, 2024
1 parent 0ab3140 commit cffa5b3
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
2 changes: 1 addition & 1 deletion eden/common/utils/ProcessInfoCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ const ProcessInfo* ProcessInfoHandle::get_optional() const {
return node_->info_.isReady() ? &node_->info_.value() : nullptr;
}

const ProcessInfo& ProcessInfoHandle::get() const {
ProcessInfo ProcessInfoHandle::get() const {
XCHECK(node_) << "attempting to use moved-from ProcessInfoHandle";
auto now = node_->clock_.now();
node_->recordAccess(now);
Expand Down
2 changes: 1 addition & 1 deletion eden/common/utils/ProcessInfoCache.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class ProcessInfoHandle {
* May throw, notably if the ProcessInfoCache is destroyed before it could
* read the process info.
*/
const ProcessInfo& get() const;
ProcessInfo get() const;

private:
FRIEND_TEST(ProcessInfoCache, faultinjector);
Expand Down

0 comments on commit cffa5b3

Please sign in to comment.