Skip to content

Commit c320f68

Browse files
haowu14facebook-github-bot
authored andcommitted
Allow navy/common/Device to take a BufferView to avoid unnecessary deep copy
Summary: Before this diff, the Device class takes ownership of the buffer to be written into the NVM device. After this diff, the Device class can take either a buffer with ownership, or a buffer view. If a buffer is used and there is encryption, the Device class will overwrite this buffer. If a buffer view is used and there is encryption, the Device class will make a copy of this buffer view and encrypt-overwrite the created buffer. A future optimization could be to allow the encryptor directly create and write the encrypted content into the new buffer. Reviewed By: jaesoo-fb Differential Revision: D42050939 fbshipit-source-id: dc0ccecb03462f43aabf645f21ccc7b245915b15
1 parent d19a01a commit c320f68

File tree

4 files changed

+27
-7
lines changed

4 files changed

+27
-7
lines changed

cachelib/navy/block_cache/RegionManager.cpp

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,7 @@ void RegionManager::reset() {
106106
Region::FlushRes RegionManager::flushBuffer(const RegionId& rid) {
107107
auto& region = getRegion(rid);
108108
auto callBack = [this](RelAddress addr, BufferView view) {
109-
auto writeBuffer = device_.makeIOBuffer(view.size());
110-
writeBuffer.copyFrom(0, view);
111-
if (!deviceWrite(addr, std::move(writeBuffer))) {
109+
if (!deviceWrite(addr, view)) {
112110
return false;
113111
}
114112
numInMemBufWaitingFlush_.dec();
@@ -488,11 +486,11 @@ bool RegionManager::isValidIORange(uint32_t offset, uint32_t size) const {
488486
return static_cast<uint64_t>(offset) + size <= regionSize_;
489487
}
490488

491-
bool RegionManager::deviceWrite(RelAddress addr, Buffer buf) {
492-
const auto bufSize = buf.size();
489+
bool RegionManager::deviceWrite(RelAddress addr, BufferView view) {
490+
const auto bufSize = view.size();
493491
XDCHECK(isValidIORange(addr.offset(), bufSize));
494492
auto physOffset = physicalOffset(addr);
495-
if (!device_.write(physOffset, std::move(buf))) {
493+
if (!device_.write(physOffset, view)) {
496494
return false;
497495
}
498496
physicalWrittenCount_.add(bufSize);

cachelib/navy/block_cache/RegionManager.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ class RegionManager {
258258
return baseOffset_ + toAbsolute(addr).offset();
259259
}
260260

261-
bool deviceWrite(RelAddress addr, Buffer buf);
261+
bool deviceWrite(RelAddress addr, BufferView buf);
262262

263263
bool isValidIORange(uint32_t offset, uint32_t size) const;
264264
OpenStatus assignBufferToRegion(RegionId rid);

cachelib/navy/common/Device.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,19 @@ class MemoryDevice final : public Device {
212212
};
213213
} // namespace
214214

215+
bool Device::write(uint64_t offset, BufferView view) {
216+
if (encryptor_) {
217+
auto writeBuffer = makeIOBuffer(view.size());
218+
writeBuffer.copyFrom(0, view);
219+
return write(offset, std::move(writeBuffer));
220+
}
221+
222+
const auto size = view.size();
223+
XDCHECK_LE(offset + size, size_);
224+
const uint8_t* data = reinterpret_cast<const uint8_t*>(view.data());
225+
return writeInternal(offset, data, size);
226+
}
227+
215228
bool Device::write(uint64_t offset, Buffer buffer) {
216229
const auto size = buffer.size();
217230
XDCHECK_LE(offset + buffer.size(), size_);
@@ -225,7 +238,10 @@ bool Device::write(uint64_t offset, Buffer buffer) {
225238
return false;
226239
}
227240
}
241+
return writeInternal(offset, data, size);
242+
}
228243

244+
bool Device::writeInternal(uint64_t offset, const uint8_t* data, size_t size) {
229245
auto remainingSize = size;
230246
auto maxWriteSize = (maxWriteSize_ == 0) ? remainingSize : maxWriteSize_;
231247
bool result = true;

cachelib/navy/common/Device.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,10 @@ class Device {
116116
// @param offset Must be ioAlignmentSize_ aligned
117117
bool write(uint64_t offset, Buffer buffer);
118118

119+
// Write buffer view to the device. This call makes a copy of the buffer if
120+
// entryptor is present.
121+
bool write(uint64_t offset, BufferView bufferView);
122+
119123
// Reads @size bytes from device at @deviceOffset and copys to @value
120124
// There must be sufficient space allocated already in the mutableView.
121125
// @offset and @size must be ioAligmentSize_ aligned
@@ -166,6 +170,8 @@ class Device {
166170

167171
bool readInternal(uint64_t offset, uint32_t size, void* value);
168172

173+
bool writeInternal(uint64_t offset, const uint8_t* data, size_t size);
174+
169175
// size of the device. All offsets for write/read should be contained
170176
// below this.
171177
const uint64_t size_{0};

0 commit comments

Comments
 (0)