Follow up for DirectIO refactor

Summary: Windows follow up for  dc2584e

Differential Revision: D4420337

Pulled By: IslamAbdelRahman

fbshipit-source-id: fedc5b5

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) {

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
 //

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;