[GR-20446] Use cached expanded load path for feature lookup (#1501).

PullRequest: truffleruby/1901

Author: bjfish

CHANGELOG.md

@@ -14,6 +14,7 @@ Bug fixes:
 * Fixed `String#encode` with options issue (#2091, #2095, @LillianZ)
 * Fixed issue with `spawn` when `:close` redirect is used (#2097).
 * Fixed `coverage` issue when `*eval` is used (#2078).
+* Use expanded load paths for feature matching (#1501).
 
 Compatibility:
 

spec/ruby/core/kernel/shared/require.rb

@@ -344,6 +344,17 @@
           loaded_feature = $LOADED_FEATURES.last
           ScratchPad.recorded.should == [loaded_feature]
         end
+
+        it "requires only once when a new matching file added to path" do
+          @object.require('load_fixture').should be_true
+          ScratchPad.recorded.should == [:loaded]
+
+          symlink_to_code_dir_two = tmp("codesymlinktwo")
+          File.symlink("#{CODE_LOADING_DIR}/b", symlink_to_code_dir_two)
+          $LOAD_PATH.unshift(symlink_to_code_dir_two)
+
+          @object.require('load_fixture').should be_false
+        end
       end
 
       describe "with symlinks in the required feature and $LOAD_PATH" do

src/main/java/org/truffleruby/core/kernel/KernelNodes.java

@@ -72,6 +72,7 @@
 import org.truffleruby.core.rope.RopeOperations;
 import org.truffleruby.core.string.RubyString;
 import org.truffleruby.core.string.StringCachingGuards;
+import org.truffleruby.core.string.StringNodes;
 import org.truffleruby.core.string.StringNodes.MakeStringNode;
 import org.truffleruby.core.string.StringOperations;
 import org.truffleruby.core.support.TypeNodes.CheckFrozenNode;
@@ -394,6 +395,19 @@ protected RubySymbol calleeName() {
         }
     }
 
+    @Primitive(name = "canonicalize_path")
+    public abstract static class CanonicalizePathNode extends PrimitiveArrayArgumentsNode {
+
+        @Specialization
+        @TruffleBoundary
+        protected RubyString canonicalPath(RubyString string,
+                @Cached StringNodes.MakeStringNode makeStringNode) {
+            final String expandedPath = getContext().getFeatureLoader().canonicalize(string.getJavaString());
+            return makeStringNode.executeMake(expandedPath, UTF8Encoding.INSTANCE, CodeRange.CR_UNKNOWN);
+        }
+
+    }
+
     @Primitive(name = "kernel_caller_locations", lowerFixnum = { 0, 1 })
     public abstract static class CallerLocationsNode extends CoreMethodArrayArgumentsNode {
 

src/main/java/org/truffleruby/core/string/RubyString.java

@@ -42,11 +42,16 @@ public RubyString(Shape shape, boolean frozen, boolean tainted, Rope rope) {
         this.rope = rope;
     }
 
+    /** should only be used for debugging */
     @Override
     public String toString() {
         return rope.toString();
     }
 
+    public String getJavaString() {
+        return StringOperations.getString(this);
+    }
+
     // region RubyLibrary messages
     @ExportMessage
     public void freeze() {

src/main/java/org/truffleruby/language/loader/FeatureLoader.java

@@ -26,6 +26,7 @@
 import org.truffleruby.RubyLanguage;
 import org.truffleruby.collections.ConcurrentOperations;
 import org.truffleruby.core.array.ArrayOperations;
+import org.truffleruby.core.array.RubyArray;
 import org.truffleruby.core.encoding.EncodingManager;
 import org.truffleruby.core.module.RubyModule;
 import org.truffleruby.core.string.RubyString;
@@ -37,6 +38,7 @@
 import org.truffleruby.interop.TranslateInteropExceptionNode;
 import org.truffleruby.language.RubyConstant;
 import org.truffleruby.language.control.RaiseException;
+import org.truffleruby.language.dispatch.DispatchNode;
 import org.truffleruby.platform.NativeConfiguration;
 import org.truffleruby.platform.Platform;
 import org.truffleruby.platform.TruffleNFIPlatform;
@@ -280,9 +282,11 @@ public String findFeatureImpl(String feature) {
         if (feature.startsWith(RubyLanguage.RESOURCE_SCHEME) || new File(feature).isAbsolute()) {
             found = findFeatureWithAndWithoutExtension(feature);
         } else if (hasExtension(feature)) {
-            for (Object pathObject : ArrayOperations.toIterable(context.getCoreLibrary().getLoadPath())) {
-                // $LOAD_PATH entries are canonicalized since Ruby 2.4.4
-                final String loadPath = canonicalize(pathObject.toString());
+            final RubyArray expandedLoadPath = (RubyArray) DispatchNode.getUncached().call(
+                    context.getCoreLibrary().truffleFeatureLoaderModule,
+                    "get_expanded_load_path");
+            for (Object pathObject : ArrayOperations.toIterable(expandedLoadPath)) {
+                final String loadPath = ((RubyString) pathObject).getJavaString();
 
                 if (context.getOptions().LOG_FEATURE_LOCATION) {
                     RubyLanguage.LOGGER.info(String.format("from load path %s...", loadPath));
@@ -298,9 +302,12 @@ public String findFeatureImpl(String feature) {
             }
         } else {
             extensionLoop: for (String extension : EXTENSIONS) {
-                for (Object pathObject : ArrayOperations.toIterable(context.getCoreLibrary().getLoadPath())) {
+                final RubyArray expandedLoadPath = (RubyArray) DispatchNode.getUncached().call(
+                        context.getCoreLibrary().truffleFeatureLoaderModule,
+                        "get_expanded_load_path");
+                for (Object pathObject : ArrayOperations.toIterable(expandedLoadPath)) {
                     // $LOAD_PATH entries are canonicalized since Ruby 2.4.4
-                    final String loadPath = canonicalize(pathObject.toString());
+                    final String loadPath = ((RubyString) pathObject).getJavaString();
 
                     if (context.getOptions().LOG_FEATURE_LOCATION) {
                         RubyLanguage.LOGGER.info(String.format("from load path %s...", loadPath));

src/main/ruby/truffleruby/core/truffle/feature_loader.rb

@@ -18,9 +18,15 @@ module FeatureLoader
     # A snapshot of $LOADED_FEATURES, to check if the @loaded_features_index cache is up to date.
     @loaded_features_copy = []
 
+    @expanded_load_path = []
+    # A snapshot of $LOAD_PATH, to check if the @expanded_load_path cache is up to date.
+    @load_path_copy = []
+
     def self.clear_cache
       @loaded_features_index.clear
       @loaded_features_copy.clear
+      @expanded_load_path.clear
+      @load_path_copy.clear
     end
 
     class FeatureEntry
@@ -149,7 +155,7 @@ def self.feature_provided?(feature, expanded)
                              if expanded
                                nil
                              else
-                               loaded_feature_path(loaded_feature, feature, $LOAD_PATH)
+                               loaded_feature_path(loaded_feature, feature, get_expanded_load_path)
                              end
                            end
             if feature_path
@@ -271,5 +277,13 @@ def self.features_index_add(feature, offset)
       end
     end
 
+    def self.get_expanded_load_path
+      unless Primitive.array_storage_equal?(@load_path_copy, $LOAD_PATH)
+        @expanded_load_path = $LOAD_PATH.map { |path| Primitive.canonicalize_path(path) }
+        @loaded_features_copy = $LOAD_PATH.dup
+      end
+      @expanded_load_path
+    end
+
   end
 end