From b51397f96c33aa421fd5c29484fb9574df9eb451 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Thu, 24 Nov 2022 16:20:57 +0100 Subject: [PATCH] Add readonly mode Fix: https://github.com/Shopify/bootsnap/issues/423 When you know that the cache won't be used again, it avoid some useless work and IOs. Typically this might be the case for dockerized applications. You generate a cache when building the image, but then when you boot the application any cache update won't be persisted, so thre is no point. --- README.md | 2 ++ ext/bootsnap/bootsnap.c | 31 ++++++++++++++++++++------- lib/bootsnap.rb | 4 ++++ lib/bootsnap/compile_cache.rb | 6 +++++- lib/bootsnap/load_path_cache.rb | 4 ++-- lib/bootsnap/load_path_cache/store.rb | 5 +++-- test/compile_cache_test.rb | 31 +++++++++++++++++++++++++++ test/setup_test.rb | 6 ++++++ 8 files changed, 76 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 04ce3972..5603031e 100644 --- a/README.md +++ b/README.md @@ -57,6 +57,7 @@ Bootsnap.setup( load_path_cache: true, # Optimize the LOAD_PATH with a cache compile_cache_iseq: true, # Compile Ruby code into ISeq cache, breaks coverage reporting. compile_cache_yaml: true, # Compile YAML into a cache + readonly: true, # Use the caches but don't update them on miss or stale entries. ) ``` @@ -77,6 +78,7 @@ well together, and are both included in a newly-generated Rails applications by - `DISABLE_BOOTSNAP` allows to entirely disable bootsnap. - `DISABLE_BOOTSNAP_LOAD_PATH_CACHE` allows to disable load path caching. - `DISABLE_BOOTSNAP_COMPILE_CACHE` allows to disable ISeq and YAML caches. +- `BOOTSNAP_READONLY` configure bootsnap to not update the cache on miss or stale entries. - `BOOTSNAP_LOG` configure bootsnap to log all caches misses to STDERR. - `BOOTSNAP_IGNORE_DIRECTORIES` a comma separated list of directories that shouldn't be scanned. Useful when you have large directories of non-ruby files inside `$LOAD_PATH`. diff --git a/ext/bootsnap/bootsnap.c b/ext/bootsnap/bootsnap.c index 208c732d..fe70e0fc 100644 --- a/ext/bootsnap/bootsnap.c +++ b/ext/bootsnap/bootsnap.c @@ -90,9 +90,11 @@ static ID instrumentation_method; static VALUE sym_miss; static VALUE sym_stale; static bool instrumentation_enabled = false; +static bool readonly = false; /* Functions exposed as module functions on Bootsnap::CompileCache::Native */ static VALUE bs_instrumentation_enabled_set(VALUE self, VALUE enabled); +static VALUE bs_readonly_set(VALUE self, VALUE enabled); static VALUE bs_compile_option_crc32_set(VALUE self, VALUE crc32_v); static VALUE bs_rb_fetch(VALUE self, VALUE cachedir_v, VALUE path_v, VALUE handler, VALUE args); static VALUE bs_rb_precompile(VALUE self, VALUE cachedir_v, VALUE path_v, VALUE handler); @@ -166,6 +168,7 @@ Init_bootsnap(void) rb_global_variable(&sym_stale); rb_define_module_function(rb_mBootsnap, "instrumentation_enabled=", bs_instrumentation_enabled_set, 1); + rb_define_module_function(rb_mBootsnap_CompileCache_Native, "readonly=", bs_readonly_set, 1); rb_define_module_function(rb_mBootsnap_CompileCache_Native, "coverage_running?", bs_rb_coverage_running, 0); rb_define_module_function(rb_mBootsnap_CompileCache_Native, "fetch", bs_rb_fetch, 4); rb_define_module_function(rb_mBootsnap_CompileCache_Native, "precompile", bs_rb_precompile, 3); @@ -182,6 +185,13 @@ bs_instrumentation_enabled_set(VALUE self, VALUE enabled) return enabled; } +static VALUE +bs_readonly_set(VALUE self, VALUE enabled) +{ + readonly = RTEST(enabled); + return enabled; +} + /* * Bootsnap's ruby code registers a hook that notifies us via this function * when compile_option changes. These changes invalidate all existing caches. @@ -945,12 +955,17 @@ try_input_to_storage(VALUE arg) static int bs_input_to_storage(VALUE handler, VALUE args, VALUE input_data, VALUE pathval, VALUE * storage_data) { - int state; - struct i2s_data i2s_data = { - .handler = handler, - .input_data = input_data, - .pathval = pathval, - }; - *storage_data = rb_protect(try_input_to_storage, (VALUE)&i2s_data, &state); - return state; + if (readonly) { + *storage_data = rb_cBootsnap_CompileCache_UNCOMPILABLE; + return 0; + } else { + int state; + struct i2s_data i2s_data = { + .handler = handler, + .input_data = input_data, + .pathval = pathval, + }; + *storage_data = rb_protect(try_input_to_storage, (VALUE)&i2s_data, &state); + return state; + } } diff --git a/lib/bootsnap.rb b/lib/bootsnap.rb index 26a628a5..03efc0d3 100644 --- a/lib/bootsnap.rb +++ b/lib/bootsnap.rb @@ -40,6 +40,7 @@ def setup( development_mode: true, load_path_cache: true, ignore_directories: nil, + readonly: false, compile_cache_iseq: true, compile_cache_yaml: true, compile_cache_json: true @@ -49,6 +50,7 @@ def setup( cache_path: "#{cache_dir}/bootsnap/load-path-cache", development_mode: development_mode, ignore_directories: ignore_directories, + readonly: readonly, ) end @@ -57,6 +59,7 @@ def setup( iseq: compile_cache_iseq, yaml: compile_cache_yaml, json: compile_cache_json, + readonly: readonly, ) end @@ -101,6 +104,7 @@ def default_setup compile_cache_iseq: !ENV["DISABLE_BOOTSNAP_COMPILE_CACHE"], compile_cache_yaml: !ENV["DISABLE_BOOTSNAP_COMPILE_CACHE"], compile_cache_json: !ENV["DISABLE_BOOTSNAP_COMPILE_CACHE"], + readonly: !!ENV["BOOTSNAP_READONLY"], ignore_directories: ignore_directories, ) diff --git a/lib/bootsnap/compile_cache.rb b/lib/bootsnap/compile_cache.rb index f33c3b9f..c73cb55f 100644 --- a/lib/bootsnap/compile_cache.rb +++ b/lib/bootsnap/compile_cache.rb @@ -10,7 +10,7 @@ def UNCOMPILABLE.inspect Error = Class.new(StandardError) PermissionError = Class.new(Error) - def self.setup(cache_dir:, iseq:, yaml:, json:) + def self.setup(cache_dir:, iseq:, yaml:, json:, readonly: false) if iseq if supported? require_relative("compile_cache/iseq") @@ -37,6 +37,10 @@ def self.setup(cache_dir:, iseq:, yaml:, json:) warn("[bootsnap/setup] JSON parsing caching is not supported on this implementation of Ruby") end end + + if supported? && defined?(Bootsnap::CompileCache::Native) + Bootsnap::CompileCache::Native.readonly = readonly + end end def self.permission_error(path) diff --git a/lib/bootsnap/load_path_cache.rb b/lib/bootsnap/load_path_cache.rb index 1b0b46da..f06c617c 100644 --- a/lib/bootsnap/load_path_cache.rb +++ b/lib/bootsnap/load_path_cache.rb @@ -28,13 +28,13 @@ class << self alias_method :enabled?, :enabled remove_method(:enabled) - def setup(cache_path:, development_mode:, ignore_directories:) + def setup(cache_path:, development_mode:, ignore_directories:, readonly: false) unless supported? warn("[bootsnap/setup] Load path caching is not supported on this implementation of Ruby") if $VERBOSE return end - store = Store.new(cache_path) + store = Store.new(cache_path, readonly: readonly) @loaded_features_index = LoadedFeaturesIndex.new diff --git a/lib/bootsnap/load_path_cache/store.rb b/lib/bootsnap/load_path_cache/store.rb index 556cbbcd..6ca696dd 100644 --- a/lib/bootsnap/load_path_cache/store.rb +++ b/lib/bootsnap/load_path_cache/store.rb @@ -13,10 +13,11 @@ class Store NestedTransactionError = Class.new(StandardError) SetOutsideTransactionNotAllowed = Class.new(StandardError) - def initialize(store_path) + def initialize(store_path, readonly: false) @store_path = store_path @txn_mutex = Mutex.new @dirty = false + @readonly = readonly load_data end @@ -63,7 +64,7 @@ def mark_for_mutation! end def commit_transaction - if @dirty + if @dirty && !@readonly dump_data @dirty = false end diff --git a/test/compile_cache_test.rb b/test/compile_cache_test.rb index e10d0f38..164b4b3a 100644 --- a/test/compile_cache_test.rb +++ b/test/compile_cache_test.rb @@ -5,6 +5,11 @@ class CompileCacheTest < Minitest::Test include(TmpdirHelper) + def teardown + super + Bootsnap::CompileCache::Native.readonly = false + end + def test_compile_option_crc32 # Just assert that this works. Bootsnap::CompileCache::Native.compile_option_crc32 = 0xffffffff @@ -112,6 +117,32 @@ def test_recache_when_size_different load(path) end + def test_dont_store_cache_after_a_miss_when_readonly + Bootsnap::CompileCache::Native.readonly = true + + path = Help.set_file("a.rb", "a = a = 3", 100) + output = RubyVM::InstructionSequence.compile_file(path) + Bootsnap::CompileCache::ISeq.expects(:input_to_storage).never + Bootsnap::CompileCache::ISeq.expects(:storage_to_output).never + Bootsnap::CompileCache::ISeq.expects(:input_to_output).once.returns(output) + + load(path) + end + + def test_dont_store_cache_after_a_stale_when_readonly + path = Help.set_file("a.rb", "a = a = 3", 100) + load(path) + + Bootsnap::CompileCache::Native.readonly = true + + output = RubyVM::InstructionSequence.compile_file(path) + Bootsnap::CompileCache::ISeq.expects(:input_to_storage).never + Bootsnap::CompileCache::ISeq.expects(:storage_to_output).once.returns(output) + Bootsnap::CompileCache::ISeq.expects(:input_to_output).never + + load(path) + end + def test_invalid_cache_file path = Help.set_file("a.rb", "a = a = 3", 100) cp = Help.cache_path("#{@tmp_dir}-iseq", path) diff --git a/test/setup_test.rb b/test/setup_test.rb index 3dbc26c3..97026bbe 100644 --- a/test/setup_test.rb +++ b/test/setup_test.rb @@ -23,6 +23,7 @@ def test_default_setup compile_cache_yaml: true, compile_cache_json: true, ignore_directories: nil, + readonly: false, ) Bootsnap.default_setup @@ -39,6 +40,7 @@ def test_default_setup_with_ENV_not_dev compile_cache_yaml: true, compile_cache_json: true, ignore_directories: nil, + readonly: false, ) Bootsnap.default_setup @@ -55,6 +57,7 @@ def test_default_setup_with_DISABLE_BOOTSNAP_LOAD_PATH_CACHE compile_cache_yaml: true, compile_cache_json: true, ignore_directories: nil, + readonly: false, ) Bootsnap.default_setup @@ -71,6 +74,7 @@ def test_default_setup_with_DISABLE_BOOTSNAP_COMPILE_CACHE compile_cache_yaml: false, compile_cache_json: false, ignore_directories: nil, + readonly: false, ) Bootsnap.default_setup @@ -94,6 +98,7 @@ def test_default_setup_with_BOOTSNAP_LOG compile_cache_yaml: true, compile_cache_json: true, ignore_directories: nil, + readonly: false, ) Bootsnap.expects(:logger=).with($stderr.method(:puts)) @@ -111,6 +116,7 @@ def test_default_setup_with_BOOTSNAP_IGNORE_DIRECTORIES compile_cache_yaml: true, compile_cache_json: true, ignore_directories: %w[foo bar], + readonly: false, ) Bootsnap.default_setup