Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Minimize the Kernel.require extra stack frames #393

Merged
merged 1 commit into from Jan 14, 2022
+52 −77
Merged

Minimize the Kernel.require extra stack frames #393

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
@@ -1,5 +1,8 @@
# Unreleased

* Minimize the Kernel.require extra stack frames. (#393)
This should reduce the noise generated by bootsnap on `LoadError`.

# 1.9.4

* Ignore absolute paths in the loaded feature index. (#385)
Expand Down
34 changes: 15 additions & 19 deletions lib/bootsnap/load_path_cache/core_ext/kernel_require.rb
View file
Open in desktop
Expand Up @@ -18,18 +18,15 @@ module Kernel

alias_method(:require_without_bootsnap, :require)

# Note that require registers to $LOADED_FEATURES while load does not.
def require_with_bootsnap_lfi(path, resolved = nil)
Bootsnap::LoadPathCache.loaded_features_index.register(path, resolved) do
require_without_bootsnap(resolved || path)
end
end

def require(path)
return false if Bootsnap::LoadPathCache.loaded_features_index.key?(path)
string_path = path.to_s
return false if Bootsnap::LoadPathCache.loaded_features_index.key?(string_path)

if (resolved = Bootsnap::LoadPathCache.load_path_cache.find(path))
return require_with_bootsnap_lfi(path, resolved)
if (resolved = Bootsnap::LoadPathCache.load_path_cache.find(string_path))
# Note that require registers to $LOADED_FEATURES while load does not.
ret = require_without_bootsnap(resolved)
Bootsnap::LoadPathCache.loaded_features_index.register(string_path, resolved)
return ret
end

raise(Bootsnap::LoadPathCache::CoreExt.make_load_error(path))
Expand All @@ -39,10 +36,13 @@ def require(path)
rescue Bootsnap::LoadPathCache::ReturnFalse
false
rescue Bootsnap::LoadPathCache::FallbackScan
fallback = true
ensure
if fallback
require_with_bootsnap_lfi(path)
if (cursor = Bootsnap::LoadPathCache.loaded_features_index.cursor(string_path))
ret = require_without_bootsnap(path)
resolved = Bootsnap::LoadPathCache.loaded_features_index.identify(string_path, cursor)
Bootsnap::LoadPathCache.loaded_features_index.register(string_path, resolved)
ret
else # If we're not given a cursor, it means we don't need to register the path (likely an absolute path)
require_without_bootsnap(path)
end
end

Expand Down Expand Up @@ -82,10 +82,6 @@ def autoload(const, path)
rescue Bootsnap::LoadPathCache::ReturnFalse
false
rescue Bootsnap::LoadPathCache::FallbackScan
fallback = true
ensure
if fallback
autoload_without_bootsnap(const, path)
end
autoload_without_bootsnap(const, path)
end
end
45 changes: 21 additions & 24 deletions lib/bootsnap/load_path_cache/loaded_features_index.rb
View file
Open in desktop
Expand Up @@ -69,6 +69,25 @@ def key?(feature)
@mutex.synchronize { @lfi.key?(feature) }
end

def cursor(short)
unless Bootsnap.absolute_path?(short.to_s)
$LOADED_FEATURES.size
end
end

def identify(short, cursor)
$LOADED_FEATURES[cursor..-1].detect do |feat|
offset = 0
while (offset = feat.index(short, offset))
if feat.index(".", offset + 1) && !feat.index("/", offset + 2)
break true
else
offset += 1
end
end
end
end

# There is a relatively uncommon case where we could miss adding an
# entry:
#
Expand All @@ -83,28 +102,8 @@ def key?(feature)
# not quite right; or
# 2. Inspect $LOADED_FEATURES upon return from yield to find the matching
# entry.
def register(short, long = nil)
# Absolute paths are not a concern.
if Bootsnap.absolute_path?(short.to_s)
return yield
end

if long.nil?
len = $LOADED_FEATURES.size
ret = yield
long = $LOADED_FEATURES[len..-1].detect do |feat|
offset = 0
while (offset = feat.index(short, offset))
if feat.index(".", offset + 1) && !feat.index("/", offset + 2)
break true
else
offset += 1
end
end
end
else
ret = yield
end
def register(short, long)
return if Bootsnap.absolute_path?(short)

hash = long.hash

Expand All @@ -123,8 +122,6 @@ def register(short, long = nil)
@lfi[short] = hash
(@lfi[altname] = hash) if altname
end

ret
end

private
Expand Down
47 changes: 13 additions & 34 deletions test/load_path_cache/loaded_features_index_test.rb
View file
Open in desktop
Expand Up @@ -15,29 +15,17 @@ def test_successful_addition
refute(@index.key?("bundler"))
refute(@index.key?("bundler.rb"))
refute(@index.key?("foo"))
@index.register("bundler", "/a/b/bundler.rb") {}
@index.register("bundler", "/a/b/bundler.rb")
assert(@index.key?("bundler"))
assert(@index.key?("bundler.rb"))
refute(@index.key?("foo"))
end

def test_no_add_on_raise
refute(@index.key?("bundler"))
refute(@index.key?("bundler.rb"))
refute(@index.key?("foo"))
assert_raises(RuntimeError) do
@index.register("bundler", "/a/b/bundler.rb") { raise }
end
refute(@index.key?("bundler"))
refute(@index.key?("bundler.rb"))
refute(@index.key?("foo"))
end

def test_infer_base_from_ext
refute(@index.key?("bundler"))
refute(@index.key?("bundler.rb"))
refute(@index.key?("foo"))
@index.register("bundler.rb") {}
@index.register("bundler.rb", nil)
assert(@index.key?("bundler"))
assert(@index.key?("bundler.rb"))
refute(@index.key?("foo"))
Expand All @@ -54,7 +42,7 @@ def test_only_strip_elidable_ext
refute(@index.key?("descriptor.rb"))
refute(@index.key?("descriptor"))
refute(@index.key?("foo"))
@index.register("descriptor.pb.rb") {}
@index.register("descriptor.pb.rb", nil)
assert(@index.key?("descriptor.pb"))
assert(@index.key?("descriptor.pb.rb"))
refute(@index.key?("descriptor.rb"))
Expand All @@ -70,7 +58,7 @@ def test_shared_library_ext_considered_elidable
refute(@index.key?("descriptor.rb"))
refute(@index.key?("descriptor"))
refute(@index.key?("foo"))
@index.register("libgit2.dylib") {}
@index.register("libgit2.dylib", nil)
assert(@index.key?("libgit2.dylib"))
refute(@index.key?("libgit2.dylib.rb"))
refute(@index.key?("libgit2.rb"))
Expand All @@ -81,7 +69,7 @@ def test_cannot_infer_ext_from_base # Current limitation
refute(@index.key?("bundler"))
refute(@index.key?("bundler.rb"))
refute(@index.key?("foo"))
@index.register("bundler") {}
@index.register("bundler", nil)
assert(@index.key?("bundler"))
refute(@index.key?("bundler.rb"))
refute(@index.key?("foo"))
Expand All @@ -91,7 +79,7 @@ def test_purge_loaded_feature
refute(@index.key?("bundler"))
refute(@index.key?("bundler.rb"))
refute(@index.key?("foo"))
@index.register("bundler", "/a/b/bundler.rb") {}
@index.register("bundler", "/a/b/bundler.rb")
assert(@index.key?("bundler"))
assert(@index.key?("bundler.rb"))
refute(@index.key?("foo"))
Expand All @@ -105,7 +93,7 @@ def test_purge_multi_loaded_feature
refute(@index.key?("bundler"))
refute(@index.key?("bundler.rb"))
refute(@index.key?("foo"))
@index.register("bundler", "/a/b/bundler.rb") {}
@index.register("bundler", "/a/b/bundler.rb")
assert(@index.key?("bundler"))
assert(@index.key?("bundler.rb"))
refute(@index.key?("foo"))
Expand All @@ -119,7 +107,10 @@ def test_register_finds_correct_feature
refute(@index.key?("bundler"))
refute(@index.key?("bundler.rb"))
refute(@index.key?("foo"))
@index.register("bundler", nil) { $LOADED_FEATURES << "/a/b/bundler.rb" }
cursor = @index.cursor("bundler")
$LOADED_FEATURES << "/a/b/bundler.rb"
long = @index.identify("bundler", cursor)
@index.register("bundler", long)
assert(@index.key?("bundler"))
assert(@index.key?("bundler.rb"))
refute(@index.key?("foo"))
Expand All @@ -136,23 +127,11 @@ def test_derives_initial_state_from_loaded_features
refute(index.key?("minitest/autorun.so"))
end

def test_works_with_pathname
path = "bundler.rb"
pathname = Pathname.new(path)
@index.register(pathname, path) { true }
assert(@index.key?(pathname))
end

def test_ignores_absolute_paths
path = "#{Dir.mktmpdir}/bundler.rb"
@index.register(path) { true }
refute(@index.key?(path))

path = "#{Dir.mktmpdir}/bundler.rb"
pathname = Pathname.new(path)
@index.register(pathname, path) { true }
assert_nil @index.cursor(path)
@index.register(path, path)
refute(@index.key?(path))
refute(@index.key?(pathname))
end
end
end
Expand Down