Vulkan-Loader icon indicating copy to clipboard operation
Vulkan-Loader copied to clipboard

Try using fseek before reading a file twice.

Open branc116 opened this issue 8 months ago • 53 comments

I was testing something with vulkan and noticed that some files are being read twice. I was supprised by that. I started digging and I found this:

commit 992d9cb45f181e747a1a942c6aa09bef94fd9428
Author: Lenny Komow <[email protected]>
Date:   Tue Dec 1 10:49:01 2020 -0700

    loader: Remove SEEK_END usage
    
    Change-Id: I699caaf048d70756649d9f6a2c7dfb012c6d2342

     }
-    fseek(file, 0, SEEK_END);
+    // NOTE: We can't just use fseek(file, 0, SEEK_END) because that isn't guaranteed to be supported on all systems
+    do {
+        // We're just seeking the end of the file, so this buffer is never used
+        char buffer[256];
+        fread(buffer, 1, sizeof(buffer), file);
+    } while (!feof(file));
     len = ftell(file);

And then I found this:

Contributor cdotstout commented on Oct 9, 2020
On Fuchsia, at one point we observed a problem where this fseek failed, from loader_get_json in loader.c:

fseek(file, 0, SEEK_END);

We fixed the fseek on Fuchsia so this is not a pressing issue, but we considered switching to fstat instead.

I noted here:
http://www.cplusplus.com/reference/cstdio/fseek/

"Library implementations are allowed to not meaningfully support SEEK_END (therefore, code using it has no real standard portability)"

At least, the fseek return code should probably be checked and an error/warning logged if it fails.

So because of new experimental os, everybody using this loader on linux, bsd, windows, ... has to read each json file 2 times. Idk, in this pull request I'd just like to use fseek and then if that fails, use double read.

I also did a quick benchmark:

NEW 3924 - 4235 - 18022 (min - avg - max microsecs)
OLD 4440 - 4810 - 19318 (min - avg - max microsecs)
#include <stdio.h>
#include <vulkan/vulkan.h>
#include <stdlib.h>

#define HS (1 << 24)
union {
  size_t data[HS];
  char c_data[HS << 3];
} heap;
size_t heap_index = 0;

void* my_alloc(void* ud, size_t size, size_t alignment, VkSystemAllocationScope allocation_scope) {
  void* ret = &heap.data[heap_index];
  heap_index += (size >> 3) + 1;
  return ret;
}

void* my_realloc(void* ud, void* p, size_t size, size_t alignment, VkSystemAllocationScope allocation_scope) {
  void* ret = &heap.data[heap_index];
  heap_index += (size >> 3) + 1;
  return ret;
}

void my_free(void* ud, void* p) {}
void alloc_notif(void* ud, size_t size, VkInternalAllocationType allocation_type, VkSystemAllocationScope allocation_scope) {}
void free_notif(void* ud, size_t size, VkInternalAllocationType allocation_type, VkSystemAllocationScope allocation_scope) {}

VkAllocationCallbacks allocator = {
  .pUserData = 0,
  .pfnAllocation = my_alloc,
  .pfnReallocation = my_realloc,
  .pfnFree = my_free
};

int main(int argc, char** argv) {
  int n = 1;
  if (argc > 1) {
    n = atoi(argv[1]);
  }
  unsigned long long total = 0;
  unsigned long long m = 1LL<<60LL, M = 0;
  for (int i = 0; i < n; ++i) {
    VkInstanceCreateInfo create_info = {0};
    VkInstance instance = {0};
    unsigned long long cur = 0;
    unsigned long long start = __builtin_ia32_rdtsc();
    vkCreateInstance(&create_info, &allocator, &instance);
    unsigned long long end = __builtin_ia32_rdtsc();
    cur = end - start;
    total += cur;
    m = m < cur ? m : cur;
    M = M < cur ? cur : M;
    heap_index = 0;
  }
  printf("%llu - %llu - %llu\n", m / 1000, total / n / 1000, M / 1000);
  return 0;
}

// cc -L $PWD/build/loader -lvulkan test.c && strace ./a.out 2> tmp
// cc -O3 -L $PWD/build/loader -lvulkan ../test.c && LD_LIBRARY_PATH="$PWD/loader" ./a.out

branc116 avatar Jun 01 '24 23:06 branc116