From 324a0f988eec725af7618af6b91f3b4b6980b497 Mon Sep 17 00:00:00 2001 From: Dmitri Smirnov Date: Sun, 15 Jan 2017 13:11:04 -0800 Subject: [PATCH] Follow up for DirectIO refactor Summary: Windows follow up for https://github.com/facebook/rocksdb/commit/dc2584eea0d9965f97d38d4d462a7b57750144d3 Differential Revision: D4420337 Pulled By: IslamAbdelRahman fbshipit-source-id: fedc5b5 --- port/win/env_win.cc | 9 +++++- port/win/io_win.cc | 40 ++++++++++++++++++++++++ port/win/io_win.h | 76 +++++++++++++++++++++++++++------------------ 3 files changed, 94 insertions(+), 31 deletions(-) diff --git a/port/win/env_win.cc b/port/win/env_win.cc index f6c58a67c95..62cc27c825a 100644 --- a/port/win/env_win.cc +++ b/port/win/env_win.cc @@ -119,13 +119,20 @@ Status WinEnvIO::NewSequentialFile(const std::string& fname, // while they are still open with another handle. For that reason we // allow share_write and delete(allows rename). HANDLE hFile = INVALID_HANDLE_VALUE; + + DWORD fileFlags = FILE_ATTRIBUTE_READONLY; + + if (options.use_direct_reads && !options.use_mmap_reads) { + fileFlags |= FILE_FLAG_NO_BUFFERING; + } + { IOSTATS_TIMER_GUARD(open_nanos); hFile = CreateFileA( fname.c_str(), GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL, OPEN_EXISTING, // Original fopen mode is "rb" - FILE_ATTRIBUTE_NORMAL, NULL); + fileFlags, NULL); } if (INVALID_HANDLE_VALUE == hFile) { diff --git a/port/win/io_win.cc b/port/win/io_win.cc index 05c863b3f2f..7c0818660b0 100644 --- a/port/win/io_win.cc +++ b/port/win/io_win.cc @@ -556,6 +556,7 @@ WinSequentialFile::~WinSequentialFile() { } Status WinSequentialFile::Read(size_t n, Slice* result, char* scratch) { + assert(result != nullptr && !WinFileData::use_direct_io()); Status s; size_t r = 0; @@ -580,6 +581,41 @@ Status WinSequentialFile::Read(size_t n, Slice* result, char* scratch) { return s; } +SSIZE_T WinSequentialFile::PositionedReadInternal(char* src, size_t numBytes, + uint64_t offset) const { + return pread(GetFileHandle(), src, numBytes, offset); +} + +Status WinSequentialFile::PositionedRead(uint64_t offset, size_t n, Slice* result, + char* scratch) { + + Status s; + + assert(WinFileData::use_direct_io()); + + // Windows ReadFile API accepts a DWORD. + // While it is possible to read in a loop if n is > UINT_MAX + // it is a highly unlikely case. + if (n > UINT_MAX) { + return IOErrorFromWindowsError(GetName(), ERROR_INVALID_PARAMETER); + } + + auto r = PositionedReadInternal(scratch, n, offset); + + if (r < 0) { + auto lastError = GetLastError(); + // Posix impl wants to treat reads from beyond + // of the file as OK. + if (lastError != ERROR_HANDLE_EOF) { + s = IOErrorFromWindowsError(GetName(), lastError); + } + } + + *result = Slice(scratch, (r < 0) ? 0 : size_t(r)); + return s; +} + + Status WinSequentialFile::Skip(uint64_t n) { // Can't handle more than signed max as SetFilePointerEx accepts a signed 64-bit // integer. As such it is a highly unlikley case to have n so large. @@ -855,6 +891,10 @@ size_t WinRandomAccessFile::GetUniqueId(char* id, size_t max_size) const { return GetUniqueIdFromFile(GetFileHandle(), id, max_size); } +size_t WinRandomAccessFile::GetRequiredBufferAlignment() const { + return GetAlignment(); +} + ///////////////////////////////////////////////////////////////////////////// // WinWritableImpl // diff --git a/port/win/io_win.h b/port/win/io_win.h index e1fde818c73..a434ab77522 100644 --- a/port/win/io_win.h +++ b/port/win/io_win.h @@ -101,6 +101,32 @@ class WinFileData { WinFileData& operator=(const WinFileData&) = delete; }; +class WinSequentialFile : protected WinFileData, public SequentialFile { + + // Override for behavior change when creating a custom env + virtual SSIZE_T PositionedReadInternal(char* src, size_t numBytes, + uint64_t offset) const; + +public: + WinSequentialFile(const std::string& fname, HANDLE f, + const EnvOptions& options); + + ~WinSequentialFile(); + + WinSequentialFile(const WinSequentialFile&) = delete; + WinSequentialFile& operator=(const WinSequentialFile&) = delete; + + virtual Status Read(size_t n, Slice* result, char* scratch) override; + virtual Status PositionedRead(uint64_t offset, size_t n, Slice* result, + char* scratch) override; + + virtual Status Skip(uint64_t n) override; + + virtual Status InvalidateCache(size_t offset, size_t length) override; + + virtual bool use_direct_io() const override { return WinFileData::use_direct_io(); } +}; + // mmap() based random-access class WinMmapReadableFile : private WinFileData, public RandomAccessFile { HANDLE hMap_; @@ -208,23 +234,6 @@ class WinMmapFile : private WinFileData, public WritableFile { virtual size_t GetUniqueId(char* id, size_t max_size) const override; }; -class WinSequentialFile : private WinFileData, public SequentialFile { - public: - WinSequentialFile(const std::string& fname, HANDLE f, - const EnvOptions& options); - - ~WinSequentialFile(); - - WinSequentialFile(const WinSequentialFile&) = delete; - WinSequentialFile& operator=(const WinSequentialFile&) = delete; - - virtual Status Read(size_t n, Slice* result, char* scratch) override; - - virtual Status Skip(uint64_t n) override; - - virtual Status InvalidateCache(size_t offset, size_t length) override; -}; - class WinRandomAccessImpl { protected: WinFileData* file_base_; @@ -281,14 +290,17 @@ class WinRandomAccessImpl { virtual ~WinRandomAccessImpl() {} - public: - WinRandomAccessImpl(const WinRandomAccessImpl&) = delete; - WinRandomAccessImpl& operator=(const WinRandomAccessImpl&) = delete; - Status ReadImpl(uint64_t offset, size_t n, Slice* result, char* scratch) const; void HintImpl(RandomAccessFile::AccessPattern pattern); + + size_t GetAlignment() const { return buffer_.Alignment(); } + + public: + + WinRandomAccessImpl(const WinRandomAccessImpl&) = delete; + WinRandomAccessImpl& operator=(const WinRandomAccessImpl&) = delete; }; // pread() based random-access @@ -303,18 +315,22 @@ class WinRandomAccessFile ~WinRandomAccessFile(); - virtual void EnableReadAhead() override; - virtual Status Read(uint64_t offset, size_t n, Slice* result, char* scratch) const override; virtual bool ShouldForwardRawRequest() const override; + virtual void EnableReadAhead() override; + + virtual size_t GetUniqueId(char* id, size_t max_size) const override; + virtual void Hint(AccessPattern pattern) override; + virtual bool use_direct_io() const override { return WinFileData::use_direct_io(); } + virtual Status InvalidateCache(size_t offset, size_t length) override; - virtual size_t GetUniqueId(char* id, size_t max_size) const override; + virtual size_t GetRequiredBufferAlignment() const override; }; // This is a sequential write class. It has been mimicked (as others) after @@ -382,12 +398,6 @@ class WinWritableFile : private WinFileData, ~WinWritableFile(); - // Indicates if the class makes use of direct I/O - // Use PositionedAppend - virtual bool use_direct_io() const override; - - virtual size_t GetRequiredBufferAlignment() const override; - virtual Status Append(const Slice& data) override; // Requires that the data is aligned as specified by @@ -408,6 +418,12 @@ class WinWritableFile : private WinFileData, virtual Status Fsync() override; + // Indicates if the class makes use of direct I/O + // Use PositionedAppend + virtual bool use_direct_io() const override; + + virtual size_t GetRequiredBufferAlignment() const override; + virtual uint64_t GetFileSize() override; virtual Status Allocate(uint64_t offset, uint64_t len) override;