Merge pull request #469 from Shopify/open-noatime

Open with `O_NOATIME`
Shopify · Jan 30, 2024 · c12002f · c12002f
2 parents 266b935 + 404745f
commit c12002f
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 23 deletions.
diff --git a/.rubocop.yml b/.rubocop.yml
@@ -64,6 +64,9 @@ Layout/RescueEnsureAlignment:
 Layout/FirstHashElementIndentation:
   EnforcedStyle: consistent
 
+Layout/FirstArrayElementIndentation:
+  EnforcedStyle: consistent
+
 Layout/SpaceInsideHashLiteralBraces:
   EnforcedStyle: no_space
 

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,6 @@
 # Unreleased
 
+* Open source files and cache entries with `O_NOATIME` when available to reduce disk accesses. See #469.
 * `bootsnap precompile --gemfile` now look for `.rb` files in the whole gem and not just the `lib/` directory. See #466.
 
 # 1.17.1

diff --git a/ext/bootsnap/bootsnap.c b/ext/bootsnap/bootsnap.c
@@ -20,6 +20,10 @@
 #include <fcntl.h>
 #include <sys/stat.h>
 
+#ifndef O_NOATIME
+#define O_NOATIME 0
+#endif
+
 /* 1000 is an arbitrary limit; FNV64 plus some slashes brings the cap down to
  * 981 for the cache dir */
 #define MAX_CACHEPATH_SIZE 1000
@@ -30,7 +34,7 @@
 #define MAX_CREATE_TEMPFILE_ATTEMPT 3
 
 #ifndef RB_UNLIKELY
-  #define RB_UNLIKELY(x) (x)
+#define RB_UNLIKELY(x) (x)
 #endif
 
 /*
@@ -366,7 +370,7 @@ open_current_file(char * path, struct bs_cache_key * key, const char ** errno_pr
   struct stat statbuf;
   int fd;
 
-  fd = open(path, O_RDONLY);
+  fd = open(path, O_RDONLY | O_NOATIME);
   if (fd < 0) {
     *errno_provenance = "bs_fetch:open_current_file:open";
     return fd;
@@ -432,7 +436,7 @@ open_cache_file(const char * path, struct bs_cache_key * key, const char ** errn
 {
   int fd, res;
 
-  fd = open(path, O_RDONLY);
+  fd = open(path, O_RDONLY | O_NOATIME);
   if (fd < 0) {
     *errno_provenance = "bs_fetch:open_cache_file:open";
     return CACHE_MISS;

diff --git a/ext/bootsnap/extconf.rb b/ext/bootsnap/extconf.rb
@@ -3,21 +3,26 @@
 require "mkmf"
 
 if %w[ruby truffleruby].include?(RUBY_ENGINE)
-  $CFLAGS << " -O3 "
-  $CFLAGS << " -std=c99"
+  unless RUBY_PLATFORM.match?(/mswin|mingw|cygwin/)
+    append_cppflags ["_GNU_SOURCE"] # Needed of O_NOATIME
+  end
+
+  append_cflags ["-O3", "-std=c99"]
 
   # ruby.h has some -Wpedantic fails in some cases
   # (e.g. https://github.com/Shopify/bootsnap/issues/15)
   unless ["0", "", nil].include?(ENV["BOOTSNAP_PEDANTIC"])
-    $CFLAGS << " -Wall"
-    $CFLAGS << " -Werror"
-    $CFLAGS << " -Wextra"
-    $CFLAGS << " -Wpedantic"
+    append_cflags([
+      "-Wall",
+      "-Werror",
+      "-Wextra",
+      "-Wpedantic",
 
-    $CFLAGS << " -Wno-unused-parameter" # VALUE self has to be there but we don't care what it is.
-    $CFLAGS << " -Wno-keyword-macro" # hiding return
-    $CFLAGS << " -Wno-gcc-compat" # ruby.h 2.6.0 on macos 10.14, dunno
-    $CFLAGS << " -Wno-compound-token-split-by-macro"
+      "-Wno-unused-parameter", # VALUE self has to be there but we don't care what it is.
+      "-Wno-keyword-macro", # hiding return
+      "-Wno-gcc-compat", # ruby.h 2.6.0 on macos 10.14, dunno
+      "-Wno-compound-token-split-by-macro",
+    ])
   end
 
   create_makefile("bootsnap/bootsnap")

diff --git a/test/compile_cache_key_format_test.rb b/test/compile_cache_key_format_test.rb
@@ -62,21 +62,20 @@ def test_key_mtime
   end
 
   def test_fetch
-    if RbConfig::CONFIG["host_os"] =~ /mswin|mingw|cygwin/
-      target = "NUL"
-      expected_file = "#{@tmp_dir}/36/9eba19c29ffe00"
-    else
-      target = "/dev/null"
-      expected_file = "#{@tmp_dir}/8c/d2d180bbd995df"
-    end
+    target = Help.set_file("a.rb", "foo = 1")
 
-    actual = Bootsnap::CompileCache::Native.fetch(@tmp_dir, target, TestHandler, nil)
+    cache_dir = File.join(@tmp_dir, "compile_cache")
+    actual = Bootsnap::CompileCache::Native.fetch(cache_dir, target, TestHandler, nil)
     assert_equal("NEATO #{target.upcase}", actual)
 
-    data = File.read(expected_file)
+    entries = Dir["#{cache_dir}/**/*"].select { |f| File.file?(f) }
+    assert_equal 1, entries.size
+    cache_file = entries.first
+
+    data = File.read(cache_file)
     assert_equal("neato #{target}", data.force_encoding(Encoding::BINARY)[64..])
 
-    actual = Bootsnap::CompileCache::Native.fetch(@tmp_dir, target, TestHandler, nil)
+    actual = Bootsnap::CompileCache::Native.fetch(cache_dir, target, TestHandler, nil)
     assert_equal("NEATO #{target.upcase}", actual)
   end