Release the autoload claim when Autoload#load has nothing to load - #2
Draft
aminmansuri wants to merge 4 commits into
Draft
Release the autoload claim when Autoload#load has nothing to load#2aminmansuri wants to merge 4 commits into
aminmansuri wants to merge 4 commits into
Conversation
A direct require of the autoload's file defines the constant while the claim is still held, so the value stays on the Autoload and the constant table keeps UNDEF; every later lookup from another thread re-enters LoadService.
ruby/spec pins what Ruby can see (remove_const value, Marshal and $LOAD_PATH independence from another thread); test/jruby pins the constant-table slot.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Autoload#loadrecords the claim (ctx) before it checksfeatureAlreadyLoaded. When the feature is already loaded or still loading there is nothing to load, but the claim is kept. Any constant that file then defines is treated as this autoload's own result and parked on theAutoload; the constant table keeps UNDEF,autoload?andconst_defined?report correctly, and every later lookup from another thread (Marshal.load,const_get, class reopening) goes throughLoadService#featureAlreadyLoadedagain instead of reading the table.The feature is "already loaded or loading" in three common shapes: a direct
requireof the autoload's file, several constants declared withautoloadagainst one file (ActiveSupport::Autoload'sautoload_at, the usual Rails layout) where the first one loaded parks its siblings, and a circularrequirebetween two autoloaded files.On the command line that walk is cheap. Under a classloader-backed
$LOAD_PATH(a WAR) each lookup costs about 450 µs; a page deserialising 89 cached objects spent 1,229 ms inMarshal.load, 36 ms once the slots held their values.The fix releases the claim in the already-loaded branch. The specs pin what Ruby can observe (a
Marshal.loadand a$LOAD_PATHchange from another thread no longer depend on the loader;remove_constreturns the value; every constant of a shared file becomes a regular constant); thetest/jrubycases pin the constant-table slot itself, including the sibling and circular shapes.