Skip to content

Commit

Permalink
let TLObserver be moveable
Browse files Browse the repository at this point in the history
Summary: It is currently copyable but not moveable, so all apparent moves are actually copies. This can be surprisingly expensive when unexpected.

Differential Revision: D57691075

fbshipit-source-id: 3f5332bb1f2767f3c01208899bffc88ac3ba2cc8
  • Loading branch information
yfeldblum authored and facebook-github-bot committed May 23, 2024
1 parent 0df8667 commit b40db01
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 7 deletions.
18 changes: 12 additions & 6 deletions third-party/folly/src/folly/observer/Observer-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -162,22 +162,28 @@ T AtomicObserver<T>::get() const {

template <typename T>
TLObserver<T>::TLObserver(Observer<T> observer)
: observer_(std::move(observer)),
snapshot_([&] { return Snapshot<T>(observer_.getSnapshot()); }) {}
: observer_(std::move(observer)) {}

template <typename T>
TLObserver<T>::TLObserver(const TLObserver<T>& other)
: TLObserver(other.observer_) {}

template <typename T>
TLObserver<T>::TLObserver(TLObserver<T>&& other) noexcept = default;

template <typename T>
const Snapshot<T>& TLObserver<T>::getSnapshotRef() const {
auto& snapshot = *snapshot_;
if (observer_.needRefresh(snapshot) ||
Snapshot<T>* snapshot = snapshot_.get();
if (FOLLY_UNLIKELY(!snapshot)) {
snapshot = new Snapshot<T>(observer_.getSnapshot());
snapshot_.reset(snapshot);
}
if (observer_.needRefresh(*snapshot) ||
observer_detail::ObserverManager::inManagerThread()) {
snapshot = observer_.getSnapshot();
*snapshot = observer_.getSnapshot();
}

return snapshot;
return *snapshot;
}

template <typename T>
Expand Down
3 changes: 2 additions & 1 deletion third-party/folly/src/folly/observer/Observer.h
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,7 @@ class TLObserver {
public:
explicit TLObserver(Observer<T> observer);
TLObserver(const TLObserver<T>& other);
TLObserver(TLObserver<T>&& other) noexcept;

const Snapshot<T>& getSnapshotRef() const;
const Snapshot<T>& operator*() const { return getSnapshotRef(); }
Expand All @@ -342,7 +343,7 @@ class TLObserver {

private:
Observer<T> observer_;
ThreadLocal<Snapshot<T>> snapshot_;
mutable ThreadLocalPtr<Snapshot<T>> snapshot_;
};

template <typename T>
Expand Down
4 changes: 4 additions & 0 deletions third-party/folly/src/folly/observer/test/ObserverTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,10 @@ TEST(Observer, TLObserver) {
EXPECT_EQ(42, ***k);
k = std::make_unique<folly::observer::TLObserver<int>>(createTLObserver(41));
EXPECT_EQ(41, ***k);
k = std::make_unique<folly::observer::TLObserver<int>>( // copy-ctor
static_cast<folly::observer::TLObserver<int> const&>(
createTLObserver(40)));
EXPECT_EQ(40, ***k);
}

TEST(ReadMostlyTLObserver, ReadMostlyTLObserver) {
Expand Down

0 comments on commit b40db01

Please sign in to comment.