Skip to content

Commit

Permalink
Adding fadvise(POSIX_FADVISE_DONTNEED) after each block write
Browse files Browse the repository at this point in the history
Summary: Adding fadvise(POSIX_FADVISE_DONTNEED) after each block write

Reviewed By: ldemailly

Differential Revision: D2947743

fb-gh-sync-id: ac5ae1faf873cbbb91f903af20d53225c10d7c34
shipit-source-id: ac5ae1faf873cbbb91f903af20d53225c10d7c34
  • Loading branch information
uddipta authored and ldemailly committed Feb 18, 2016
1 parent 9c925d2 commit 9edeae0
Show file tree
Hide file tree
Showing 9 changed files with 27 additions and 3 deletions.
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ include(CheckCXXSourceCompiles)
check_function_exists(posix_fallocate HAS_POSIX_FALLOCATE)
check_function_exists(sync_file_range HAS_SYNC_FILE_RANGE)
check_function_exists(posix_memalign HAS_POSIX_MEMALIGN)
check_function_exists(posix_fadvise HAS_POSIX_FADVISE)
# C based check (which fail with the c++ setting thereafter...)
check_library_exists(rt clock_gettime "" FOLLY_HAVE_CLOCK_GETTIME)
set(SAVE_CMRL ${CMAKE_REQUIRED_LIBRARIES}) #globals are evil/ugly
Expand Down
2 changes: 1 addition & 1 deletion Reporting.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ void ProgressReporter::logProgress(int64_t effectiveDataBytes, int progress,
const std::string PerfStatReport::statTypeDescription_[] = {
"Socket Read", "Socket Write", "File Open", "File Close", "File Read",
"File Write", "Sync File Range", "fsync", "File Seek", "Throttler Sleep",
"Receiver Wait Sleep", "Directory creation", "Ioctl", "Unlink"};
"Receiver Wait Sleep", "Directory creation", "Ioctl", "Unlink", "Fadvise"};

PerfStatReport::PerfStatReport(const WdtOptions& options) {
static_assert(
Expand Down
1 change: 1 addition & 0 deletions Reporting.h
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,7 @@ class PerfStatReport {
DIRECTORY_CREATE,
IOCTL,
UNLINK,
FADVISE,
END
};

Expand Down
2 changes: 2 additions & 0 deletions WdtConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
#define HAS_POSIX_FALLOCATE 1
#define HAS_SYNC_FILE_RANGE 1
#define HAS_POSIX_MEMALIGN 1
#define HAS_POSIX_FADVISE 1

#define WDT_SUPPORTS_ODIRECT 1
#define WDT_HAS_SOCKIOS_H 1
// Again do not add new defines here without editing WdtConfig.h.in ...
1 change: 1 addition & 0 deletions WdtConfig.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#cmakedefine HAS_POSIX_FALLOCATE 1
#cmakedefine HAS_SYNC_FILE_RANGE 1
#cmakedefine HAS_POSIX_MEMALIGN 1
#cmakedefine HAS_POSIX_FADVISE 1

#if (defined(HAS_POSIX_MEMALIGN) && defined(O_DIRECT)) || defined(F_NOCACHE)
#define WDT_SUPPORTS_ODIRECT 1
Expand Down
1 change: 1 addition & 0 deletions WdtOptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ void WdtOptions::modifyOptions(
msg)
CHANGE_IF_NOT_SPECIFIED(resume_using_dir_tree, userSpecifiedOptions, true,
msg)
CHANGE_IF_NOT_SPECIFIED(skip_fadvise, userSpecifiedOptions, true, msg)
return;
}
if (optionType != FLASH_OPTION_TYPE) {
Expand Down
5 changes: 5 additions & 0 deletions WdtOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,11 @@ class WdtOptions {
*/
bool delete_extra_files{false};

/**
* If true, fadvise is skipped after block write
*/
bool skip_fadvise{false};

/**
* @return whether files should be pre-allocated or not
*/
Expand Down
16 changes: 14 additions & 2 deletions util/FileWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ void FileWriter::close() {

ErrorCode FileWriter::write(char *buf, int64_t size) {
WDT_CHECK_NE(TO_BE_DELETED, blockDetails_->allocationStatus);
if (!threadCtx_.getOptions().skip_writes) {
auto &options = threadCtx_.getOptions();
if (!options.skip_writes) {
int64_t count = 0;
while (count < size) {
int64_t written;
Expand All @@ -83,7 +84,7 @@ ErrorCode FileWriter::write(char *buf, int64_t size) {
VLOG(1) << "Successfully written " << count << " bytes to fd " << fd_
<< " for file " << blockDetails_->fileName;
bool finished = ((totalWritten_ + size) == blockDetails_->dataSize);
if (threadCtx_.getOptions().isLogBasedResumption() && finished) {
if (finished && options.isLogBasedResumption()) {
PerfStatCollector statCollector(threadCtx_, PerfStatReport::FSYNC);
if (fsync(fd_) != 0) {
PLOG(ERROR) << "fsync failed for " << blockDetails_->fileName
Expand All @@ -95,6 +96,17 @@ ErrorCode FileWriter::write(char *buf, int64_t size) {
} else {
syncFileRange(count, finished);
}
#ifdef HAS_POSIX_FADVISE
if (finished && !options.skip_fadvise) {
PerfStatCollector statCollector(threadCtx_, PerfStatReport::FADVISE);
if (posix_fadvise(fd_, blockDetails_->offset, blockDetails_->dataSize,
POSIX_FADV_DONTNEED) != 0) {
PLOG(ERROR) << "posix_fadvise failed for " << blockDetails_->fileName
<< " " << blockDetails_->offset << " "
<< blockDetails_->dataSize;
}
}
#endif
}
totalWritten_ += size;
return OK;
Expand Down
1 change: 1 addition & 0 deletions util/WdtFlags.cpp.inc
Original file line number Diff line number Diff line change
Expand Up @@ -170,3 +170,4 @@ WDT_OPT(receive_buffer_size, int32,
WDT_OPT(
delete_extra_files, bool,
"If true, extra files on the receiver side is deleted during resumption");
WDT_OPT(skip_fadvise, bool, "If true, fadvise is skipped after block write");

0 comments on commit 9edeae0

Please sign in to comment.