pitrou commented on code in PR #13640:
URL: https://github.com/apache/arrow/pull/13640#discussion_r961812146


##########
cpp/src/arrow/io/file.cc:
##########
@@ -378,6 +378,101 @@ Status FileOutputStream::Write(const void* data, int64_t 
length) {
 
 int FileOutputStream::file_descriptor() const { return impl_->fd(); }
 
+// ----------------------------------------------------------------------
+// DirectFileOutputStream, change the Open, Write and Close methods from 
FileOutputStream
+// Uses DirectIO for writes. Will only write out things in 4096 byte blocks. 
Buffers
+// leftover bytes in an internal data structure, which will be padded to 4096 
bytes and
+// flushed upon call to close.
+
+class DirectFileOutputStream::DirectFileOutputStreamImpl : public OSFile {
+ public:
+  Status Open(const std::string& path, bool append) {
+    const bool truncate = !append;
+    return OpenWritable(path, truncate, append, true /* write_only */, true);
+  }
+  Status Open(int fd) { return OpenWritable(fd); }
+};
+
+DirectFileOutputStream::DirectFileOutputStream() {
+  uintptr_t mask = (uintptr_t)(4095);

Review Comment:
   > But this only works if you know what the path is. So I don't know how 
you'd be able to support and Open from just a file descriptor. I will probably 
have to change that API.
   
   Oh, then let's forget about it and keep the hard-coded 4096 (should be good 
enough I suppose?).
   
   However, please make it a named constant instead of repeating the magic 
number everywhere, e.g.:
   ```c++
   // The minimum write size and alighment for direct I/O.
   // Unfortunately there's no easy way to query the actual number
   // even at runtime (it depends on the kernel and filesystem version),
   // so we hard-code a reasonable value instead.
   static constexpr int64_t kDirectIoMinimumSize = 4096;
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to