Merge pull request #25034 from JuliaLang/jn/require_ji_better2

StefanKarpinski · web-flow · commit 7d92d3d9e316 · 2017-12-18T13:58:03.000-05:00
loading: refactor .ji loading, to handle errors better
diff --git a/base/loading.jl b/base/loading.jl
@@ -118,22 +118,58 @@ end
 
 # these return either the array of modules loaded from the path / content given
 # or an Exception that describes why it couldn't be loaded
-function _include_from_serialized(content::Vector{UInt8})
-    return ccall(:jl_restore_incremental_from_buf, Any, (Ptr{UInt8}, Int), content, sizeof(content))
+function _include_from_serialized(content::Vector{UInt8}, depmods::Vector{Module})
+    return ccall(:jl_restore_incremental_from_buf, Any, (Ptr{UInt8}, Int, Any), content, sizeof(content), depmods)
 end
-function _include_from_serialized(path::String)
-    return ccall(:jl_restore_incremental, Any, (Cstring,), path)
+function _include_from_serialized(path::String, depmods::Vector{Module})
+    return ccall(:jl_restore_incremental, Any, (Cstring, Any), path, depmods)
 end
 
 # returns an array of modules loaded, or an Exception that describes why it failed
 # and it reconnects the Base.Docs.META
 function _require_from_serialized(mod::Symbol, path_to_try::String)
-    restored = _include_from_serialized(path_to_try)
+    return _require_from_serialized(mod, path_to_try, parse_cache_header(path_to_try)[3])
+end
+function _require_from_serialized(mod::Symbol, path_to_try::String, depmodnames::Vector{Pair{Symbol, UInt64}})
+    # load all of the dependent modules
+    ndeps = length(depmodnames)
+    depmods = Vector{Module}(uninitialized, ndeps)
+    for i in 1:ndeps
+        modname, uuid = depmodnames[i]
+        if root_module_exists(modname)
+            M = root_module(modname)
+            if module_name(M) === modname && module_uuid(M) === uuid
+                depmods[i] = M
+            end
+        else
+            modpath = find_package(string(modname))
+            modpath === nothing && return ErrorException("Required dependency $modname not found in current path.")
+            mod = _require_search_from_serialized(modname, String(modpath))
+            if !isa(mod, Bool)
+                for M in mod::Vector{Any}
+                    if module_name(M) === modname && module_uuid(M) === uuid
+                        depmods[i] = M
+                        break
+                    end
+                end
+                for callback in package_callbacks
+                    invokelatest(callback, modname)
+                end
+            end
+        end
+        isassigned(depmods, i) || return ErrorException("Required dependency $modname failed to load from a cache file.")
+    end
+    # now load the path_to_try.ji file
+    restored = _include_from_serialized(path_to_try, depmods)
     if !isa(restored, Exception)
         for M in restored::Vector{Any}
+            M = M::Module
             if isdefined(M, Base.Docs.META)
                 push!(Base.Docs.modules, M)
             end
+            if module_parent(M) === M
+                register_root_module(module_name(M), M)
+            end
         end
     end
     return restored
@@ -145,12 +181,13 @@ end
 function _require_search_from_serialized(mod::Symbol, sourcepath::String)
     paths = find_all_in_cache_path(mod)
     for path_to_try in paths::Vector{String}
-        if stale_cachefile(sourcepath, path_to_try)
+        deps = stale_cachefile(sourcepath, path_to_try)
+        if deps === true
             continue
         end
-        restored = _require_from_serialized(mod, path_to_try)
+        restored = _require_from_serialized(mod, path_to_try, deps)
         if isa(restored, Exception)
-            if isa(restored, ErrorException) && endswith(restored.msg, " uuid did not match cache file.")
+            if isa(restored, ErrorException)
                 # can't use this cache due to a module uuid mismatch,
                 # defer reporting error until after trying all of the possible matches
                 @debug "Failed to load $path_to_try because $(restored.msg)"
@@ -178,7 +215,7 @@ const package_callbacks = Any[]
 const include_callbacks = Any[]
 
 # used to optionally track dependencies when requiring a module:
-const _concrete_dependencies = Any[] # these dependency versions are "set in stone", and the process should try to avoid invalidating them
+const _concrete_dependencies = Pair{Symbol, UInt64}[] # these dependency versions are "set in stone", and the process should try to avoid invalidating them
 const _require_dependencies = Any[] # a list of (mod, path, mtime) tuples that are the file dependencies of the module currently being precompiled
 const _track_dependencies = Ref(false) # set this to true to track the list of file dependencies
 function _include_dependency(modstring::AbstractString, _path::AbstractString)
@@ -358,14 +395,6 @@ function unreference_module(key)
     end
 end
 
-function register_all(a)
-    for m in a
-        if module_parent(m) === m
-            register_root_module(module_name(m), m)
-        end
-    end
-end
-
 function _require(mod::Symbol)
     # dependency-tracking is only used for one top-level include(path),
     # and is not applied recursively to imported modules:
@@ -390,13 +419,13 @@ function _require(mod::Symbol)
         if path === nothing
             throw(ArgumentError("Module $name not found in current path.\nRun `Pkg.add(\"$name\")` to install the $name package."))
         end
+        path = String(path)
 
         # attempt to load the module file via the precompile cache locations
         doneprecompile = false
         if JLOptions().use_compiled_modules != 0
             doneprecompile = _require_search_from_serialized(mod, path)
             if !isa(doneprecompile, Bool)
-                register_all(doneprecompile)
                 return
             end
         end
@@ -423,7 +452,6 @@ function _require(mod::Symbol)
                 @warn "The call to compilecache failed to create a usable precompiled cache file for module $name" exception=m
                 # fall-through, TODO: disable __precompile__(true) error so that the normal include will succeed
             else
-                register_all(m)
                 return
             end
         end
@@ -446,7 +474,6 @@ function _require(mod::Symbol)
                 # TODO: disable __precompile__(true) error and do normal include instead of error
                 error("Module $mod declares __precompile__(true) but require failed to create a usable precompiled cache file.")
             end
-            register_all(m)
         end
     finally
         toplevel_load[] = last
@@ -539,7 +566,7 @@ function evalfile(path::AbstractString, args::Vector{String}=String[])
 end
 evalfile(path::AbstractString, args::Vector) = evalfile(path, String[args...])
 
-function create_expr_cache(input::String, output::String, concrete_deps::Vector{Any})
+function create_expr_cache(input::String, output::String, concrete_deps::typeof(_concrete_dependencies))
     rm(output, force=true)   # Remove file if it exists
     code_object = """
         while !eof(STDIN)
@@ -606,12 +633,12 @@ function compilecache(name::String)
     if !isdir(cachepath)
         mkpath(cachepath)
     end
-    cachefile::String = abspath(cachepath, name*".ji")
+    cachefile::String = abspath(cachepath, "$name.ji")
     # build up the list of modules that we want the precompile process to preserve
     concrete_deps = copy(_concrete_dependencies)
-    for (key,mod) in loaded_modules
+    for (key, mod) in loaded_modules
         if !(mod === Main || mod === Core || mod === Base)
-            push!(concrete_deps, (key, module_uuid(mod)))
+            push!(concrete_deps, key => module_uuid(mod))
         end
     end
     # run the expression and cache the result
@@ -637,13 +664,13 @@ module_uuid(m::Module) = ccall(:jl_module_uuid, UInt64, (Any,), m)
 isvalid_cache_header(f::IOStream) = 0 != ccall(:jl_read_verify_header, Cint, (Ptr{Void},), f.ios)
 
 function parse_cache_header(f::IO)
-    modules = Dict{Symbol,UInt64}()
+    modules = Vector{Pair{Symbol, UInt64}}()
     while true
         n = ntoh(read(f, Int32))
         n == 0 && break
         sym = Symbol(read(f, n)) # module symbol
         uuid = ntoh(read(f, UInt64)) # module UUID (mostly just a timestamp)
-        modules[sym] = uuid
+        push!(modules, sym => uuid)
     end
     totbytes = ntoh(read(f, Int64)) # total bytes for file dependencies
     # read the list of files
@@ -662,13 +689,13 @@ function parse_cache_header(f::IO)
     @assert totbytes == 12 "header of cache file appears to be corrupt"
     srctextpos = ntoh(read(f, Int64))
     # read the list of modules that are required to be present during loading
-    required_modules = Dict{Symbol,UInt64}()
+    required_modules = Vector{Pair{Symbol, UInt64}}()
     while true
         n = ntoh(read(f, Int32))
         n == 0 && break
         sym = Symbol(read(f, n)) # module symbol
         uuid = ntoh(read(f, UInt64)) # module UUID
-        required_modules[sym] = uuid
+        push!(required_modules, sym => uuid)
     end
     return modules, files, required_modules, srctextpos
 end
@@ -729,6 +756,8 @@ function read_dependency_src(cachefile::String, filename::AbstractString)
     end
 end
 
+# returns true if it "cachefile.ji" is stale relative to "modpath.jl"
+# otherwise returns the list of dependencies to also check
 function stale_cachefile(modpath::String, cachefile::String)
     io = open(cachefile, "r")
     try
@@ -737,13 +766,12 @@ function stale_cachefile(modpath::String, cachefile::String)
             return true # invalid cache file
         end
         modules, files, required_modules = parse_cache_header(io)
+        modules = Dict{Symbol, UInt64}(modules)
 
         # Check if transitive dependencies can be fullfilled
-        for mod in keys(required_modules)
-            if mod == :Main || mod == :Core || mod == :Base
-                continue
+        for (mod, uuid_req) in required_modules
             # Module is already loaded
-            elseif root_module_exists(mod)
+            if root_module_exists(mod)
                 continue
             end
             name = string(mod)
@@ -761,7 +789,7 @@ function stale_cachefile(modpath::String, cachefile::String)
             uuid = get(modules, mod, UInt64(0))
             if uuid !== UInt64(0)
                 if uuid === uuid_req
-                    return false # this is the file we want
+                    return required_modules # this is the file we want
                 end
                 @debug "Rejecting cache file $cachefile because it provides the wrong uuid (got $uuid) for $mod (want $uuid_req)"
                 return true # cachefile doesn't provide the required version of the dependency
@@ -790,7 +818,7 @@ function stale_cachefile(modpath::String, cachefile::String)
             return true
         end
 
-        return false # fresh cachefile
+        return required_modules # fresh cachefile
     finally
         close(io)
     end
diff --git a/base/precompile.jl b/base/precompile.jl
@@ -1417,7 +1417,7 @@ precompile(Tuple{typeof(Base.collect_to!), Array{Int32, 1}, Base.Generator{Array
 precompile(Tuple{typeof(Base.collect_to!), Array{Symbol, 1}, Base.Generator{Array{Any, 1}, typeof(Base.FastMath.make_fastmath)}, Int64, Int64})
 precompile(Tuple{typeof(Base.compilecache), String})
 precompile(Tuple{typeof(Base.copyto!), Array{Tuple{String, Float64}, 1}, Int64, Array{Tuple{String, Float64}, 1}, Int64, Int64})
-precompile(Tuple{typeof(Base.create_expr_cache), String, String, Array{Any, 1}})
+precompile(Tuple{typeof(Base.create_expr_cache), String, String, typeof(Base._concrete_dependencies)})
 precompile(Tuple{typeof(Base._delete!), Base.Dict{Symbol, Base.Condition}, Int64})
 precompile(Tuple{typeof(Base.Docs.docm), LineNumberNode, Module, String, Expr})
 precompile(Tuple{typeof(Base.Docs.docm), LineNumberNode, Module, String, Expr, Bool})
@@ -1444,7 +1444,7 @@ precompile(Tuple{typeof(Base.ht_keyindex), Base.Dict{Symbol, UInt64}, Symbol})
 precompile(Tuple{typeof(Base.ht_keyindex), Base.Dict{Tuple{String, Float64}, Void}, Tuple{String, Float64}})
 precompile(Tuple{typeof(Base.ident_cmp), Tuple{String, String, Int64}, Tuple{String, Int64}})
 precompile(Tuple{typeof(Base.include_relative), String})
-precompile(Tuple{typeof(Base._include_from_serialized), String})
+precompile(Tuple{typeof(Base._include_from_serialized), String, Vector{Module}})
 precompile(Tuple{typeof(Base.indexed_next), Tuple{Symbol, UInt64}, Int64, Int64})
 precompile(Tuple{typeof(Base.indexed_next), Tuple{Void, Void}, Int64, Int64})
 precompile(Tuple{typeof(Base.isassigned), Array{String, 1}, Int64})
@@ -1485,7 +1485,7 @@ precompile(Tuple{typeof(Base.rehash!), Base.Dict{Symbol, Base.Condition}, Int64}
 precompile(Tuple{typeof(Base.rehash!), Base.Dict{Symbol, UInt64}, Int64})
 precompile(Tuple{typeof(Base.rehash!), Base.Dict{Tuple{String, Float64}, Void}, Int64})
 precompile(Tuple{typeof(Base.remove_linenums!), Module})
-precompile(Tuple{typeof(Base._require_from_serialized), Symbol, String})
+precompile(Tuple{typeof(Base._require_from_serialized), Symbol, String, Vector{Pair{Symbol, UInt64}}})
 precompile(Tuple{typeof(Base._require_search_from_serialized), Symbol, String})
 precompile(Tuple{typeof(Base.require), Symbol})
 precompile(Tuple{typeof(Base.resize!), Array{Base.Condition, 1}, Int64})
diff --git a/src/dump.c b/src/dump.c
@@ -1924,53 +1924,29 @@ static int size_isgreater(const void *a, const void *b)
     return *(size_t*)b - *(size_t*)a;
 }
 
-static jl_value_t *read_verify_mod_list(ios_t *s, arraylist_t *dependent_worlds)
+static jl_value_t *read_verify_mod_list(ios_t *s, arraylist_t *dependent_worlds, jl_array_t *mod_list)
 {
     if (!jl_main_module->uuid) {
         return jl_get_exceptionf(jl_errorexception_type,
                 "Main module uuid state is invalid for module deserialization.");
     }
-    jl_array_t *mod_array = jl_alloc_vec_any(0);
-    JL_GC_PUSH1(&mod_array);
-    while (1) {
+    size_t i, l = jl_array_len(mod_list);
+    for (i = 0; ; i++) {
         size_t len = read_int32(s);
-        if (len == 0) {
-            JL_GC_POP();
-            return (jl_value_t*)mod_array;
-        }
-        char *name = (char*)alloca(len+1);
+        if (len == 0 && i == l)
+            return NULL; // success
+        if (len == 0 || i == l)
+            return jl_get_exceptionf(jl_errorexception_type, "Wrong number of entries in module list.");
+        char *name = (char*)alloca(len + 1);
         ios_read(s, name, len);
         name[len] = '\0';
         uint64_t uuid = read_uint64(s);
-        jl_sym_t *sym = jl_symbol(name);
-        jl_module_t *m = NULL;
-        static jl_value_t *require_func = NULL;
-        if (!require_func)
-            require_func = jl_get_global(jl_base_module, jl_symbol("require"));
-        jl_value_t *reqargs[2] = {require_func, (jl_value_t*)sym};
-        JL_TRY {
-            m = (jl_module_t*)jl_apply(reqargs, 2);
-        }
-        JL_CATCH {
-            ios_close(s);
-            jl_rethrow();
-        }
-        if (!m) {
-            JL_GC_POP();
-            return jl_get_exceptionf(jl_errorexception_type,
-                    "Requiring \"%s\" did not define a corresponding module.", name);
-        }
-        if (!jl_is_module(m)) {
-            JL_GC_POP();
-            return jl_get_exceptionf(jl_errorexception_type,
-                "Invalid module path (%s does not name a module).", name);
-        }
-        if (m->uuid != uuid) {
-            JL_GC_POP();
+        jl_sym_t *sym = jl_symbol_n(name, len);
+        jl_module_t *m = (jl_module_t*)jl_array_ptr_ref(mod_list, i);
+        if (!m || !jl_is_module(m) || m->name != sym || m->uuid != uuid) {
             return jl_get_exceptionf(jl_errorexception_type,
-                "Module %s uuid did not match cache file.", name);
+                "Invalid input in module list: expected %s.", name);
         }
-        jl_array_ptr_1d_push(mod_array, (jl_value_t*)m);
         if (m->primary_world > jl_main_module->primary_world)
             arraylist_push(dependent_worlds, (void*)m->primary_world);
     }
@@ -2659,7 +2635,7 @@ static int trace_method(jl_typemap_entry_t *entry, void *closure)
     return 1;
 }
 
-static jl_value_t *_jl_restore_incremental(ios_t *f)
+static jl_value_t *_jl_restore_incremental(ios_t *f, jl_array_t *mod_array)
 {
     jl_ptls_t ptls = jl_get_ptls_states();
     if (ios_eof(f) || !jl_read_verify_header(f)) {
@@ -2682,13 +2658,12 @@ static jl_value_t *_jl_restore_incremental(ios_t *f)
     arraylist_new(&dependent_worlds, 0);
 
     // verify that the system state is valid
-    jl_value_t *verify_result = read_verify_mod_list(f, &dependent_worlds);
-    if (!jl_is_array(verify_result)) {
+    jl_value_t *verify_fail = read_verify_mod_list(f, &dependent_worlds, mod_array);
+    if (verify_fail) {
         arraylist_free(&dependent_worlds);
         ios_close(f);
-        return verify_result;
+        return verify_fail;
     }
-    jl_array_t *mod_array = (jl_array_t*)verify_result;
 
     // prepare to deserialize
     int en = jl_gc_enable(0);
@@ -2751,21 +2726,21 @@ static jl_value_t *_jl_restore_incremental(ios_t *f)
     return (jl_value_t*)restored;
 }
 
-JL_DLLEXPORT jl_value_t *jl_restore_incremental_from_buf(const char *buf, size_t sz)
+JL_DLLEXPORT jl_value_t *jl_restore_incremental_from_buf(const char *buf, size_t sz, jl_array_t *mod_array)
 {
     ios_t f;
     ios_static_buffer(&f, (char*)buf, sz);
-    return _jl_restore_incremental(&f);
+    return _jl_restore_incremental(&f, mod_array);
 }
 
-JL_DLLEXPORT jl_value_t *jl_restore_incremental(const char *fname)
+JL_DLLEXPORT jl_value_t *jl_restore_incremental(const char *fname, jl_array_t *mod_array)
 {
     ios_t f;
     if (ios_file(&f, fname, 1, 0, 0, 0) == NULL) {
         return jl_get_exceptionf(jl_errorexception_type,
             "Cache file \"%s\" not found.\n", fname);
     }
-    return _jl_restore_incremental(&f);
+    return _jl_restore_incremental(&f, mod_array);
 }
 
 // --- init ---
diff --git a/src/julia.h b/src/julia.h
@@ -1381,8 +1381,8 @@ JL_DLLEXPORT void jl_save_system_image(const char *fname);
 JL_DLLEXPORT void jl_restore_system_image(const char *fname);
 JL_DLLEXPORT void jl_restore_system_image_data(const char *buf, size_t len);
 JL_DLLEXPORT int jl_save_incremental(const char *fname, jl_array_t *worklist);
-JL_DLLEXPORT jl_value_t *jl_restore_incremental(const char *fname);
-JL_DLLEXPORT jl_value_t *jl_restore_incremental_from_buf(const char *buf, size_t sz);
+JL_DLLEXPORT jl_value_t *jl_restore_incremental(const char *fname, jl_array_t *depmods);
+JL_DLLEXPORT jl_value_t *jl_restore_incremental_from_buf(const char *buf, size_t sz, jl_array_t *depmods);
 
 // front end interface
 JL_DLLEXPORT jl_value_t *jl_parse_input_line(const char *str, size_t len,
diff --git a/test/compile.jl b/test/compile.jl
