Skip to content

Commit 32a022f

Browse files
mubashirqcopybara-github
authored andcommitted
[peregrine] Add ResolveAddrCallback and base-address resolution in RequestTracker.
PiperOrigin-RevId: 1008474233
1 parent 37f63e0 commit 32a022f

17 files changed

Lines changed: 198 additions & 57 deletions

‎peregrine/src/api/transport_test.cc‎

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -58,17 +58,23 @@ class TransportTest : public ::testing::TestWithParam<Param> {
5858
completed_status_(Status::kNotFound),
5959
partial_(kBufSize / 2),
6060
l_(kBufSize, kNumConnsPerPeer),
61-
r_(kBufSize, kNumConnsPerPeer,
62-
[this](Status s, const Metadata& post_meta,
63-
absl::Span<const Metadata> req_metas) {
64-
absl::MutexLock lock(cb_mu_);
65-
recv_posts_.push_back(PostEvent{
66-
.status = s,
67-
.post_meta = post_meta,
68-
.req_metas =
69-
std::vector<Metadata>(req_metas.begin(), req_metas.end()),
70-
});
71-
}) {
61+
r_(
62+
kBufSize, kNumConnsPerPeer,
63+
[this](Status s, const Metadata& post_meta,
64+
absl::Span<const Metadata> req_metas) {
65+
absl::MutexLock lock(cb_mu_);
66+
recv_posts_.push_back(PostEvent{
67+
.status = s,
68+
.post_meta = post_meta,
69+
.req_metas =
70+
std::vector<Metadata>(req_metas.begin(), req_metas.end()),
71+
});
72+
},
73+
[this](const Metadata& req_meta) -> Byte* {
74+
if (req_meta == Metadata{1, 2, 3}) return r_.DataPtr();
75+
if (req_meta == Metadata{4, 5, 6}) return r_.DataPtr() + partial_;
76+
return nullptr;
77+
}) {
7278
CHECK(1 <= partial_ && partial_ < l_.DataSize());
7379
DCHECK_EQ(l_.DataSize(), r_.DataSize());
7480
}
@@ -172,7 +178,7 @@ TEST_P(TransportTest, Write) {
172178
const Request req2 = {
173179
.op = Op::kWrite,
174180
.laddr = l_.DataPtr() + partial_,
175-
.raddr = r_.DataPtr() + partial_,
181+
.raddr = nullptr,
176182
.len = l_.DataSize() - partial_,
177183
.metadata = {4, 5, 6},
178184
};

‎peregrine/src/api/transport_types.h‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,7 @@ struct Request final {
5151
Metadata metadata = {}; // optional opaque per-request metadata
5252

5353
// Returns true iff the request is valid.
54-
constexpr bool IsValid() const {
55-
return laddr != nullptr && raddr != nullptr && 1 <= len;
56-
}
54+
constexpr bool IsValid() const { return laddr != nullptr && 1 <= len; }
5755

5856
// Returns true iff the requests are equal.
5957
friend bool operator==(const Request& a, const Request& b) {
@@ -85,6 +83,11 @@ using RecvPostCallback =
8583
absl::AnyInvocable<void(Status, const Metadata& post_metadata,
8684
absl::Span<const Metadata> req_metadata) const>;
8785

86+
// Callback invoked on the receiver to resolve the target buffer address for an
87+
// incoming request when `Request::raddr` is not provided by the sender.
88+
using ResolveAddrCallback =
89+
absl::AnyInvocable<Byte*(const Metadata& req_metadata) const>;
90+
8891
// Returns true iff the transport operation is still in progress.
8992
constexpr bool IsInProgress(const Status s) { return s == Status::kInProgress; }
9093

‎peregrine/src/api/transport_types_test.cc‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,15 @@ TEST(TransportRequest, Validity) {
6262
EXPECT_TRUE(rv.IsValid());
6363
EXPECT_TRUE(IsValid({rv}));
6464
LOG(INFO) << rv;
65+
66+
const Request rv_null_raddr = {
67+
.op = Op::kWrite,
68+
.laddr = reinterpret_cast<Byte*>(0x1234),
69+
.raddr = nullptr,
70+
.len = 1,
71+
};
72+
EXPECT_TRUE(rv_null_raddr.IsValid());
73+
EXPECT_TRUE(IsValid({rv_null_raddr}));
6574
}
6675

6776
TEST(TransportRequest, RKeyAndEquality) {

‎peregrine/src/api/transport_util.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@ std::unique_ptr<Transport> CreateTransport(std::string_view endpoint,
3737
};
3838

3939
return TransportImpl::Create(config, e, std::move(creds),
40-
std::move(options.recv_post_callback));
40+
std::move(options.recv_post_callback),
41+
std::move(options.resolve_addr_callback));
4142
}
4243

4344
} // namespace peregrine

‎peregrine/src/api/transport_util.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@ struct Options {
2020
// Callback invoked on the receiver when an incoming `Post()` batch completes.
2121
// If `nullptr`, no callback is invoked on post receive completion.
2222
RecvPostCallback recv_post_callback = nullptr;
23+
24+
// Callback invoked on the receiver to resolve the target buffer address for
25+
// an incoming request when `Request::raddr` is not provided by the sender.
26+
// If `nullptr`, incoming requests with a null `raddr` are rejected.
27+
ResolveAddrCallback resolve_addr_callback = nullptr;
2328
};
2429

2530
// Creates a new transport instance that listens for control messages on

‎peregrine/src/internal/chunk/header.h‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,6 @@ struct alignas(8) DataHeader final : public HeaderBase {
122122
// Returns true iff this `DataHeader` is valid.
123123
bool IsValid() const { return IsData() || IsAck(); }
124124

125-
// Returns the destination memory address for this chunk.
126-
Byte* DstAddr() const { return reinterpret_cast<Byte*>(raddr.value()); }
127-
128125
// Returns a string representation for debugging/logging.
129126
std::string ToString() const;
130127

@@ -189,8 +186,8 @@ struct alignas(8) RequestInfo final {
189186

190187
// Returns true iff this `RequestInfo` is valid.
191188
bool IsValid() const {
192-
return (op == Op::kRead || op == Op::kWrite) && raddr.value() != 0 &&
193-
total_len > 0 && nchunks > 0 && recipe.IsValid();
189+
return (op == Op::kRead || op == Op::kWrite) && total_len > 0 &&
190+
nchunks > 0 && recipe.IsValid();
194191
}
195192

196193
// Returns a string representation for debugging/logging.

‎peregrine/src/internal/chunk/header_test.cc‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ TEST(HeaderTest, PostInfoDataAndAckProperties) {
4242
EXPECT_TRUE(data.IsData());
4343
EXPECT_FALSE(data.IsAck());
4444
EXPECT_TRUE(data.IsValid());
45-
EXPECT_EQ(data.DstAddr(), reinterpret_cast<Byte*>(kBufferBaseAddr.value()));
4645
EXPECT_TRUE(IsMatch(data, GenPayload(kChunkSize)));
4746
EXPECT_FALSE(IsMatch(data, GenPayload(kChunkSize - 1)));
4847
EXPECT_EQ(data, data);
@@ -75,6 +74,9 @@ TEST(RequestInfoTest, ValidationAndEquality) {
7574
EXPECT_TRUE(req.IsValid());
7675
EXPECT_EQ(req, req);
7776
LOG(INFO) << req;
77+
78+
req.raddr = addr_t(0);
79+
EXPECT_TRUE(req.IsValid());
7880
}
7981

8082
} // namespace

‎peregrine/src/internal/engine/engine.cc‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,10 @@ absl::Status NotFoundError(const Handle h) {
5151
}
5252
} // namespace
5353

54-
std::unique_ptr<Engine> Engine::Create(const Config& config, HostInfo& self,
55-
Control& control,
56-
RecvPostCallback recv_post_callback) {
54+
std::unique_ptr<Engine> Engine::Create(
55+
const Config& config, HostInfo& self, Control& control,
56+
RecvPostCallback recv_post_callback,
57+
ResolveAddrCallback resolve_addr_callback) {
5758
static_assert(assumptions::kHostInfoDependsOnControlAndDataPlanes);
5859

5960
if ABSL_PREDICT_FALSE (!config.IsValid()) {
@@ -67,20 +68,23 @@ std::unique_ptr<Engine> Engine::Create(const Config& config, HostInfo& self,
6768
}
6869
DCHECK(self.IsDataPlaneValid());
6970
return absl::WrapUnique(new Engine(config, self, std::move(helper),
70-
std::move(recv_post_callback)));
71+
std::move(recv_post_callback),
72+
std::move(resolve_addr_callback)));
7173
}
7274

7375
Engine::Engine(const Config& config, const HostInfo& self,
7476
std::unique_ptr<EngineHelper> helper,
75-
RecvPostCallback recv_post_callback)
77+
RecvPostCallback recv_post_callback,
78+
ResolveAddrCallback resolve_addr_callback)
7679
: config_(config),
7780
self_(self),
7881
stop_(false),
7982
next_handle_(util::Random<Handle::ValueType>(bitgen_)),
8083
next_reqid_(util::Random<ReqId::ValueType>(bitgen_)),
8184
outgoing_(helper->Metrics(), helper->Tracer()),
8285
incoming_(helper->Metrics(), helper->Tracer(),
83-
std::move(recv_post_callback)),
86+
std::move(recv_post_callback),
87+
std::move(resolve_addr_callback)),
8488
metrics_(helper->Metrics()),
8589
tracer_(helper->Tracer()),
8690
helper_(std::move(helper)) {
@@ -291,7 +295,9 @@ void Engine::processWrite(Workers& workers, const Handle handle,
291295
chunk.reqid = req_info.reqid;
292296
chunk.index = chunk_t(i);
293297
chunk.byte_offset = offset;
294-
chunk.raddr = addr_t(req_info.raddr.value() + offset);
298+
chunk.raddr = req_info.raddr.value() == 0
299+
? addr_t(0)
300+
: addr_t(req_info.raddr.value() + offset);
295301
if ABSL_PREDICT_FALSE (trace_ctx.is_traced) {
296302
chunk.trace.SetTraced();
297303
}

‎peregrine/src/internal/engine/engine.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@ class Engine final {
6868
// Creates an engine.
6969
static std::unique_ptr<Engine> Create(
7070
const Config& config, HostInfo& self, Control& control,
71-
RecvPostCallback recv_post_callback = nullptr);
71+
RecvPostCallback recv_post_callback = nullptr,
72+
ResolveAddrCallback resolve_addr_callback = nullptr);
7273

7374
// Disallows copy and move.
7475
DISALLOW_COPY(Engine);
@@ -102,7 +103,8 @@ class Engine final {
102103
// Constructor.
103104
Engine(const Config& config, const HostInfo& self,
104105
std::unique_ptr<EngineHelper> helper,
105-
RecvPostCallback recv_post_callback);
106+
RecvPostCallback recv_post_callback,
107+
ResolveAddrCallback resolve_addr_callback);
106108

107109
// Generates a unique handle.
108110
Handle genHandle() ABSL_EXCLUSIVE_LOCKS_REQUIRED(mu_) {

‎peregrine/src/internal/request/request_tracker.cc‎

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,30 @@ bool RequestTracker::Add(const Handle handle,
5858
const Metadata& metadata) {
5959
DCHECK(!reqs.empty());
6060
absl::MutexLock _(trackers_mu_);
61-
PostTracker& pt = trackers_[handle];
61+
auto [it, inserted] = trackers_.try_emplace(handle);
62+
PostTracker& pt = it->second;
6263
if (!pt.req_map.empty()) return true;
6364
pt.metadata = metadata;
6465
pt.pending_reqs = reqs.size();
6566
pt.req_map.reserve(reqs.size());
6667
for (const RequestInfo& req : reqs) {
6768
DCHECK(req.IsValid());
69+
addr_t raddr = req.raddr;
70+
if (raddr.value() == 0 && resolve_addr_callback_ != nullptr) {
71+
raddr = addr_t(
72+
reinterpret_cast<uintptr_t>(resolve_addr_callback_(req.metadata)));
73+
}
74+
if (pt.start_time == PostTracker::kInvalidTime && raddr.value() == 0) {
75+
if (inserted) {
76+
trackers_.erase(it);
77+
} else {
78+
pt.req_map.clear();
79+
pt.pending_reqs = 0;
80+
}
81+
return false;
82+
}
6883
pt.req_map[req.reqid] =
69-
std::make_unique<ReqTracker>(req.nchunks, req.metadata);
84+
std::make_unique<ReqTracker>(req.nchunks, raddr, req.metadata);
7085
}
7186
return true;
7287
}
@@ -138,6 +153,18 @@ void RequestTracker::Update(const Handle handle, const ReqId reqid,
138153
}
139154
}
140155

156+
Byte* RequestTracker::GetChunkAddr(const Handle handle, const ReqId reqid,
157+
const uint64_t byte_offset) const {
158+
absl::MutexLock _(trackers_mu_);
159+
const auto it = trackers_.find(handle);
160+
if ABSL_PREDICT_FALSE (it == trackers_.end()) return nullptr;
161+
const auto req_it = it->second.req_map.find(reqid);
162+
if ABSL_PREDICT_FALSE (req_it == it->second.req_map.end()) return nullptr;
163+
const uintptr_t base = req_it->second->raddr.value();
164+
if ABSL_PREDICT_FALSE (base == 0) return nullptr;
165+
return reinterpret_cast<Byte*>(base + byte_offset);
166+
}
167+
141168
ChunkTracker& RequestTracker::Find(const Handle handle, const ReqId reqid) {
142169
absl::MutexLock _(trackers_mu_);
143170
return trackers_.at(handle).req_map.at(reqid)->chunks;

0 commit comments

Comments
 (0)