Add write buffering to Windows SDL_IOStream (#12424) (#14794)

src/io/SDL_iostream.c

@@ -70,6 +70,9 @@ typedef struct IOStreamWindowsData
     void *data;
     size_t size;
     size_t left;
+    void *write_data;
+    size_t write_pos;
+    bool writable;
     bool append;
     bool autoclose;
 } IOStreamWindowsData;
@@ -80,7 +83,8 @@ typedef struct IOStreamWindowsData
 #define INVALID_SET_FILE_POINTER 0xFFFFFFFF
 #endif
 
-#define READAHEAD_BUFFER_SIZE 1024
+#define READAHEAD_BUFFER_SIZE   1024
+#define WRITEBEHIND_BUFFER_SIZE 1024
 
 static HANDLE SDLCALL windows_file_open(const char *filename, const char *mode)
 {
@@ -150,6 +154,41 @@ static HANDLE SDLCALL windows_file_open(const char *filename, const char *mode)
     return h;
 }
 
+static bool windows_flush_write_buffer(IOStreamWindowsData *iodata,
+                                        SDL_IOStatus *status)
+{
+    if (iodata->write_pos == 0) {
+        return true;  // Nothing to flush
+    }
+
+    // In append mode, seek to EOF before writing
+    if (iodata->append) {
+        LARGE_INTEGER windowsoffset;
+        windowsoffset.QuadPart = 0;
+        if (!SetFilePointerEx(iodata->h, windowsoffset,
+                              &windowsoffset, FILE_END)) {
+            if (status) {
+                *status = SDL_IO_STATUS_ERROR;
+            }
+            WIN_SetError("Error seeking in datastream");
+            return false;
+        }
+    }
+
+    DWORD bytes;
+    if (!WriteFile(iodata->h, iodata->write_data,
+                   (DWORD)iodata->write_pos, &bytes, NULL)) {
+        if (status) {
+            *status = SDL_IO_STATUS_ERROR;
+        }
+        WIN_SetError("Error writing to datastream");
+        return false;
+    }
+
+    iodata->write_pos = 0;
+    return true;
+}
+
 static Sint64 SDLCALL windows_file_size(void *userdata)
 {
     IOStreamWindowsData *iodata = (IOStreamWindowsData *) userdata;
@@ -168,6 +207,10 @@ static Sint64 SDLCALL windows_file_seek(void *userdata, Sint64 offset, SDL_IOWhe
     DWORD windowswhence;
     LARGE_INTEGER windowsoffset;
 
+    if (!windows_flush_write_buffer(iodata, NULL)) {
+        return -1;
+    }
+
     // FIXME: We may be able to satisfy the seek within buffered data
     if ((whence == SDL_IO_SEEK_CUR) && (iodata->left)) {
         offset -= iodata->left;
@@ -204,6 +247,10 @@ static size_t SDLCALL windows_file_read(void *userdata, void *ptr, size_t size,
     size_t read_ahead;
     DWORD bytes;
 
+    if (!windows_flush_write_buffer(iodata, status)) {
+        return 0;
+    }
+
     if (iodata->left > 0) {
         void *data = (char *)iodata->data +
                      iodata->size -
@@ -270,11 +317,20 @@ static size_t SDLCALL windows_file_read(void *userdata, void *ptr, size_t size,
 
 static size_t SDLCALL windows_file_write(void *userdata, const void *ptr, size_t size, SDL_IOStatus *status)
 {
-    IOStreamWindowsData *iodata = (IOStreamWindowsData *) userdata;
+    IOStreamWindowsData *iodata = (IOStreamWindowsData *)userdata;
-    DWORD bytes;
+    const Uint8 *src = (const Uint8 *)ptr;
+    size_t remaining = size;
+    size_t total_written = 0;
 
+    if (!iodata->writable) {
+        *status = SDL_IO_STATUS_READONLY;
+        return 0;
+    }
+
+    // Invalidate read-ahead buffer if it has data
     if (iodata->left) {
-        if (!SetFilePointer(iodata->h, -(LONG)iodata->left, NULL, FILE_CURRENT)) {
+        if (!SetFilePointer(iodata->h, -(LONG)iodata->left,
+                            NULL, FILE_CURRENT)) {
             *status = SDL_IO_STATUS_ERROR;
             WIN_SetError("Error seeking in datastream");
             return 0;
@@ -282,30 +338,66 @@ static size_t SDLCALL windows_file_write(void *userdata, const void *ptr, size_t
         iodata->left = 0;
     }
 
-    // if in append mode, we must go to the EOF before write
+    // For large writes, flush buffer and write directly
-    if (iodata->append) {
+    if (size >= WRITEBEHIND_BUFFER_SIZE) {
-        LARGE_INTEGER windowsoffset;
+        if (!windows_flush_write_buffer(iodata, status)) {
-        windowsoffset.QuadPart = 0;
-        if (!SetFilePointerEx(iodata->h, windowsoffset, &windowsoffset, FILE_END)) {
-            *status = SDL_IO_STATUS_ERROR;
-            WIN_SetError("Error seeking in datastream");
             return 0;
         }
+
+        // In append mode, seek to EOF before direct write
+        if (iodata->append) {
+            LARGE_INTEGER windowsoffset;
+            windowsoffset.QuadPart = 0;
+            if (!SetFilePointerEx(iodata->h, windowsoffset,
+                                  &windowsoffset, FILE_END)) {
+                *status = SDL_IO_STATUS_ERROR;
+                WIN_SetError("Error seeking in datastream");
+                return 0;
+            }
+        }
+
+        DWORD bytes;
+        if (!WriteFile(iodata->h, ptr, (DWORD)size, &bytes, NULL)) {
+            *status = SDL_IO_STATUS_ERROR;
+            WIN_SetError("Error writing to datastream");
+            return 0;
+        } else if (bytes == 0 && size > 0) {
+            *status = SDL_IO_STATUS_NOT_READY;
+        }
+        return bytes;
+    }
+
+    // Buffer small writes
+    while (remaining > 0) {
+        size_t space = WRITEBEHIND_BUFFER_SIZE - iodata->write_pos;
+        size_t to_buffer = SDL_min(remaining, space);
+
+        SDL_memcpy((char *)iodata->write_data + iodata->write_pos,
+                   src, to_buffer);
+        iodata->write_pos += to_buffer;
+        src += to_buffer;
+        remaining -= to_buffer;
+        total_written += to_buffer;
+
+        if (iodata->write_pos == WRITEBEHIND_BUFFER_SIZE) {
+            if (!windows_flush_write_buffer(iodata, status)) {
+                return total_written;
+            }
+        }
     }
 
-    if (!WriteFile(iodata->h, ptr, (DWORD)size, &bytes, NULL)) {
+    return total_written;
-        *status = SDL_IO_STATUS_ERROR;
-        WIN_SetError("Error writing to datastream");
-        return 0;
-    } else if (bytes == 0 && size > 0) {
-        *status = SDL_IO_STATUS_NOT_READY;
-    }
-    return bytes;
 }
 
 static bool SDLCALL windows_file_flush(void *userdata, SDL_IOStatus *status)
 {
-    IOStreamWindowsData *iodata = (IOStreamWindowsData *) userdata;
+    IOStreamWindowsData *iodata = (IOStreamWindowsData *)userdata;
+
+    if (!windows_flush_write_buffer(iodata, status)) {
+        return false;
+    }
+
+    // Sync to disk
     if (!FlushFileBuffers(iodata->h)) {
         return WIN_SetError("Error flushing datastream");
     }
@@ -314,16 +406,23 @@ static bool SDLCALL windows_file_flush(void *userdata, SDL_IOStatus *status)
 
 static bool SDLCALL windows_file_close(void *userdata)
 {
-    IOStreamWindowsData *iodata = (IOStreamWindowsData *) userdata;
+    IOStreamWindowsData *iodata = (IOStreamWindowsData *)userdata;
+    bool result = true;
+
+    if (!windows_flush_write_buffer(iodata, NULL)) {
+        result = false;
+    }
+
     if (iodata->h != INVALID_HANDLE_VALUE) {
         if (iodata->autoclose) {
             CloseHandle(iodata->h);
         }
-        iodata->h = INVALID_HANDLE_VALUE; // to be sure
+        iodata->h = INVALID_HANDLE_VALUE;
     }
     SDL_free(iodata->data);
+    SDL_free(iodata->write_data);
     SDL_free(iodata);
-    return true;
+    return result;
 }
 
 SDL_IOStream *SDL_IOFromHandle(HANDLE handle, const char *mode, bool autoclose)
@@ -348,6 +447,9 @@ SDL_IOStream *SDL_IOFromHandle(HANDLE handle, const char *mode, bool autoclose)
     iface.close = windows_file_close;
 
     iodata->h = handle;
+    iodata->writable = (SDL_strchr(mode, 'w') != NULL) ||
+                       (SDL_strchr(mode, 'a') != NULL) ||
+                       (SDL_strchr(mode, '+') != NULL);
     iodata->append = (SDL_strchr(mode, 'a') != NULL);
     iodata->autoclose = autoclose;
 
@@ -357,6 +459,13 @@ SDL_IOStream *SDL_IOFromHandle(HANDLE handle, const char *mode, bool autoclose)
         return NULL;
     }
 
+    iodata->write_data = (char *)SDL_malloc(WRITEBEHIND_BUFFER_SIZE);
+    if (!iodata->write_data) {
+        iface.close(iodata);
+        return NULL;
+    }
+    iodata->write_pos = 0;
+
     SDL_IOStream *iostr = SDL_OpenIO(&iface, iodata);
     if (!iostr) {
         iface.close(iodata);