Skip to content

Commit 2da0a2a

Browse files
minsiimeta-codesync[bot]
authored andcommitted
Fix F14Map warning at oss build
Summary: See title. Before fix, we see many of the following warning in NCCL OSS build: ``` warning: ‘std::enable_if_t<(! folly::Conjunction<std::is_nothrow_move_constructible<T>, std::is_nothrow_destructible<_Up> >::value)> folly::f14::detail::VectorContainerPolicy<Key, MappedTypeOrVoid, HasherOrVoid, KeyEqualOrVoid, AllocOrVoid, EligibleForPerturbedInsertionOrder>::complainUnlessNothrowMoveAndDestroy() [with T = std::deque<std::unique_ptr<PendingOp> >; Key = int; MappedTypeOrVoid = std::deque<std::unique_ptr<PendingOp> >; HasherOrVoid = void; KeyEqualOrVoid = void; AllocOrVoid = void; EligibleForPerturbedInsertionOrder = std::integral_constant<bool, true>; std::enable_if_t<(! folly::Conjunction<std::is_nothrow_move_constructible<T>, std::is_nothrow_destructible<_Up> >::value)> = void]’ is deprecated: mark {key_type,mapped_type} {move constructor,destructor} noexcept, or use F14Node* if they aren't" ``` Example: https://www.internalfb.com/sandcastle/workflow/2783224569726899686/artifact/actionlog.2783224569921182489.stdout.1?selectedLines=19953-19953-56-877 This patch fixes it Reviewed By: elvinlife Differential Revision: D87721250 fbshipit-source-id: b235af7d1ca737bc2e17cd92700f6329210d7000
1 parent 4d97555 commit 2da0a2a

File tree

3 files changed

+34
-17
lines changed

3 files changed

+34
-17
lines changed

comms/ctran/backends/ib/CtranIb.h

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -655,9 +655,9 @@ class CtranIb {
655655
if (it == rankToPendingOpsMap.end()) {
656656
// create a new entry for the peer if it does not exist and VC is not
657657
// established
658-
rankToPendingOpsMap[peerRank].push_back(std::move(pendingOp));
658+
rankToPendingOpsMap[peerRank].q.push_back(std::move(pendingOp));
659659
} else {
660-
it->second.push_back(std::move(pendingOp));
660+
it->second.q.push_back(std::move(pendingOp));
661661
}
662662
pending = true;
663663
}
@@ -686,7 +686,7 @@ class CtranIb {
686686
// If VC has established, move the pendingOps list to readyToIssueOps;
687687
// otherwise, skip and move to next peer.
688688
if (vc) {
689-
for (auto& op : it->second) {
689+
for (auto& op : it->second.q) {
690690
readyToIssueOps[peerRank].push_back(std::move(op));
691691
}
692692
// Remove the peer entry once all pending ops are issued.
@@ -1116,8 +1116,15 @@ class CtranIb {
11161116
std::unique_ptr<::ctran::ib::LocalVirtualConn> localVc;
11171117
std::mutex localVcMutex;
11181118

1119-
folly::F14FastMap<int, std::deque<std::unique_ptr<PendingOp>>>
1120-
rankToPendingOpsMap;
1119+
struct PendingOpQueue {
1120+
std::deque<std::unique_ptr<PendingOp>> q;
1121+
PendingOpQueue() = default;
1122+
PendingOpQueue(const PendingOpQueue&) noexcept = default;
1123+
PendingOpQueue(PendingOpQueue&&) noexcept = default;
1124+
~PendingOpQueue() noexcept = default;
1125+
};
1126+
1127+
folly::F14FastMap<int, PendingOpQueue> rankToPendingOpsMap;
11211128
std::mutex pendingOpsMutex;
11221129

11231130
CtranCtrlManager* ctrlMgr{nullptr};

comms/ctran/backends/socket/CtranSocket.cc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -335,10 +335,10 @@ bool CtranSocket::addToPendingOpsIfRequired(
335335

336336
if (it == locked->end()) {
337337
// if there's no socket and no pendingops queue, create a queue
338-
locked->emplace(peerRank, std::deque<std::unique_ptr<SockPendingOp>>());
339-
locked->at(peerRank).push_back(std::move(pendingOp));
338+
auto res = locked->emplace(peerRank, PendingOpQueue());
339+
res.first->second.q.push_back(std::move(pendingOp));
340340
} else {
341-
it->second.push_back(std::move(pendingOp));
341+
it->second.q.push_back(std::move(pendingOp));
342342
}
343343
pending = true;
344344
}
@@ -367,7 +367,7 @@ commResult_t CtranSocket::progressPendingOps(void) {
367367
// If Socket has established, move the pendingOps list to
368368
// readyToIssueOps; otherwise, skip and move to next peer.
369369
if (sock) {
370-
for (auto& op : it->second) {
370+
for (auto& op : it->second.q) {
371371
readyToIssueOps[peerRank].push_back(std::move(op));
372372
}
373373
// Remove the peer entry once all pending ops are issued.

comms/ctran/backends/socket/CtranSocket.h

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,11 @@ class CtranSocket {
9696
struct recvCtrlQueue {
9797
std::deque<std::unique_ptr<SockPendingOp>> postedOps_;
9898
std::deque<std::unique_ptr<ControlMsg>> unexpMsgs_;
99+
100+
recvCtrlQueue() = default;
101+
recvCtrlQueue(recvCtrlQueue&& other) noexcept = default;
102+
recvCtrlQueue(const recvCtrlQueue& other) = default;
103+
~recvCtrlQueue() noexcept = default;
99104
};
100105

101106
void init(const SocketServerAddr& serverAddr);
@@ -170,12 +175,9 @@ class CtranSocket {
170175
}
171176

172177
inline recvCtrlQueue& getRecvCtrlQueue(int peerRank) {
173-
auto it = rankToRecvCtrlMap_.find(peerRank);
174-
if (it == rankToRecvCtrlMap_.end()) {
175-
rankToRecvCtrlMap_[peerRank] = recvCtrlQueue();
176-
}
177-
auto& recvQueue = rankToRecvCtrlMap_[peerRank];
178-
return recvQueue;
178+
auto [it, inserted] =
179+
rankToRecvCtrlMap_.try_emplace(peerRank, recvCtrlQueue());
180+
return it->second;
179181
}
180182

181183
const int rank_;
@@ -198,8 +200,16 @@ class CtranSocket {
198200
// the rankToSocket is accessed by both the main thread and the listen thread.
199201
folly::Synchronized<SocketMaps> socketMaps_;
200202

201-
folly::Synchronized<
202-
folly::F14FastMap<int, std::deque<std::unique_ptr<SockPendingOp>>>>
203+
struct PendingOpQueue {
204+
std::deque<std::unique_ptr<SockPendingOp>> q;
205+
206+
PendingOpQueue() = default;
207+
PendingOpQueue(const PendingOpQueue&) noexcept = default;
208+
PendingOpQueue(PendingOpQueue&&) noexcept = default;
209+
~PendingOpQueue() noexcept = default;
210+
};
211+
212+
folly::Synchronized<folly::F14FastMap<int, PendingOpQueue>>
203213
rankToPendingOpsMap_;
204214

205215
// every rank maintains a postedrecv queue and unexpected msg queue

0 commit comments

Comments
 (0)