Remove costly io/resource lookup

The `immutable-source-file?` check is used in `class-info` to know if it makes
sense to invalidate a cache entry based on modification time. This check uses
the `:file` it is given to see if the resource is inside a jar or not.

This `:path` comes from `orchard.java.{parser,legacy-parser}`, and is the
relative path on the classpath. This is then looked up via `io/resource`, so
that the URL can be checked to see if it's a jar or not.

This resource check is expensive, depending on circumstances cider-nrepl's
"stacktrace" analysis can get extremely slow, around 10~20 seconds to analyze
all stack frames and return a result, which means that the error buffer will
only pop up 10~20 seconds after evaluating a form which causes an error. The
check is performed whether we have a cached result or not.

However java.parser already calls `io/resource` to resolve the full resource
URL, so we should be able to use the information we already have. However, the
way java.parser was doing this caused inconsistencies between filesystem
resources vs archived resources.

It does a `(.getPath (io/resource path))`, and stores that under `:file`. This
works for regular files, because `.getPath` on a `file:` URL returns the path of
the file. For `jar:` URLs it returns the nested url+the jar entry, so a `:file`
URL but with a dangling `!/jar/entry`. In other words the `:file` entry is
either an actual file path, or something that looks like a `file:` URL but
isn't.

This patch
- Makes the result from java.parser consistent: `:path` is a relative classpath
  resource path, `:file` is a file on the filesystem path. Either it points at the
  resource, or at the archive containing the resource.
- Add an `archive?` flag to the result of `source-info`, so we don't have to
  figure out again later if it's an archived resource or not
- Changes the check in `class-info` to be a cheap lookup of two map keys. If
  it's an archive or if we don't have a filesystem path, then don't bother
  checking the lastModified time.

Illustration of why calling `getPath` on `jar:` URLs is not useful:

```clojure
(io/resource "lambdaisland/witchcraft.clj")
;;=> #java.net.URL "file:/srv/mc/witchcraft/src/lambdaisland/witchcraft.clj"

(.getPath (io/resource "lambdaisland/witchcraft.clj"))
;;=> "/srv/mc/witchcraft/src/lambdaisland/witchcraft.clj"

(io/resource "clojure/lang/RT.class")
;;=> #java.net.URL "jar:file:/root/.m2/repository/org/clojure/clojure/1.10.3/clojure-1.10.3.jar!/clojure/lang/RT.class"

(.getPath (io/resource "clojure/lang/RT.class"))
;;=> "file:/root/.m2/repository/org/clojure/clojure/1.10.3/clojure-1.10.3.jar!/clojure/lang/RT.class"
```

This has chopped of the `jar:` prefix, but not the `!/jar/file/entry` suffix.
The result is a `file:` URL that can't be resolved.

CHANGELOG.md

@@ -9,6 +9,7 @@
 ### Bugs Fixed
 
 * [#123](https://github.com/clojure-emacs/orchard/pull/123): Fix info lookups from namespaces that don't yet exist
+* [#124](https://github.com/clojure-emacs/orchard/pull/124): Remove costly `io/resource` lookup
 
 ## 0.7.1 (2021-04-18)
 

src/orchard/java.clj

@@ -250,16 +250,6 @@
 
 (def cache (atom {}))
 
-(defn- immutable-source-file?
-  "Return true if the source file is effectively immutable. Specifically, this
-  returns true if no source file is available, or if the source file is in a
-  jar/zip archive."
-  [info]
-  (let [path (:file info)
-        src  (when path (io/resource path))]
-    (or (not src)
-        (re-find #"\.(jar|zip)!" (str src)))))
-
 (defn class-info
   "For the class symbol, return (possibly cached) Java class and member info.
   Members are indexed first by name, and then by argument types to list all
@@ -269,7 +259,7 @@
         info (if cached
                (:info cached)
                (class-info* class))
-        last-modified (if (immutable-source-file? info)
+        last-modified (if (or (:archive? info) (nil? (:path info)))
                         0
                         (.lastModified ^File (io/file (:path info))))
         stale (not= last-modified (:last-modified cached))

src/orchard/java/legacy_parser.clj

@@ -13,6 +13,7 @@
                           ModifierFilter RootDocImpl)
    (com.sun.tools.javac.tree JCTree)
    (java.io StringReader)
+   (java.net URL JarURLConnection)
    (java.net URI)
    (javax.swing.text.html HTML$Tag HTMLEditorKit$ParserCallback)
    (javax.swing.text.html.parser ParserDelegator)
@@ -260,6 +261,18 @@
       (str/replace "." "/")
       (str ".java")))
 
+(def url-protocol (memfn ^URL getProtocol))
+
+(defn jar-path
+  "Given a jar:file:...!/... URL, return the location of the jar file on the
+  filesystem. Returns nil on any other URL."
+  [^URL jar-resource]
+  (when (= "jar" (url-protocol jar-resource))
+    (let [^JarURLConnection conn (.openConnection jar-resource)
+          inner-url (.getJarFileURL conn)]
+      (when (= "file" (url-protocol inner-url))
+        (.getPath inner-url)))))
+
 (defn source-info
   "If the source for the Java class is available on the classpath, parse it
   and return info to supplement reflection. Specifically, this includes source
@@ -270,9 +283,18 @@
   (try
     (let [path (source-path klass)]
       (when-let [root (parse-java path)]
-        (assoc (->> (map parse-info (.classes root))
-                    (filter #(= klass (:class %)))
-                    (first))
-               :file path
-               :path (. (io/resource path) getPath))))
+        (let [resource-url (io/resource path)
+              protocol (url-protocol resource-url)]
+          (assoc (->> (map parse-info (.classes root))
+                      (filter #(= klass (:class %)))
+                      (first))
+                 ;; relative path on the classpath
+                 :file path
+                 ;; filesystem object for this resource, either the file
+                 ;; itself, or its jar archive
+                 :path (case protocol
+                         "file" (.getPath resource-url)
+                         "jar" (jar-path resource-url))
+                 ;; is the resource inside an archive
+                 :archive? (= "jar" protocol)))))
     (catch Abort _)))

src/orchard/java/parser.clj

@@ -7,6 +7,7 @@
    [clojure.string :as str])
   (:import
    (java.io StringReader StringWriter)
+   (java.net URL JarURLConnection)
    (javax.lang.model.element Element ElementKind ExecutableElement
                              TypeElement VariableElement)
    (javax.swing.text.html HTML$Tag HTMLEditorKit$ParserCallback)
@@ -285,6 +286,18 @@
         (str module "/" path)
         path))))
 
+(def url-protocol (memfn ^URL getProtocol))
+
+(defn jar-path
+  "Given a jar:file:...!/... URL, return the location of the jar file on the
+  filesystem. Returns nil on any other URL."
+  [^URL jar-resource]
+  (when (= "jar" (url-protocol jar-resource))
+    (let [^JarURLConnection conn (.openConnection jar-resource)
+          inner-url (.getJarFileURL conn)]
+      (when (= "file" (url-protocol inner-url))
+        (.getPath inner-url)))))
+
 (defn source-info
   "If the source for the Java class is available on the classpath, parse it
   and return info to supplement reflection. Specifically, this includes source
@@ -296,15 +309,24 @@
     (when-let [path (source-path klass)]
       (when-let [^DocletEnvironment root (parse-java path (module-name klass))]
         (try
-          (assoc (->> (.getIncludedElements root)
-                      (filter #(#{ElementKind/CLASS
-                                  ElementKind/INTERFACE
-                                  ElementKind/ENUM}
-                                (.getKind ^Element %)))
-                      (map #(parse-info % root))
-                      (filter #(= klass (:class %)))
-                      (first))
-                 :file path
-                 :path (.getPath (io/resource path)))
+          (let [resource-url (io/resource path)
+                protocol (url-protocol resource-url)]
+            (assoc (->> (.getIncludedElements root)
+                        (filter #(#{ElementKind/CLASS
+                                    ElementKind/INTERFACE
+                                    ElementKind/ENUM}
+                                  (.getKind ^Element %)))
+                        (map #(parse-info % root))
+                        (filter #(= klass (:class %)))
+                        (first))
+                   ;; relative path on the classpath
+                   :file path
+                   ;; filesystem object for this resource, either the file
+                   ;; itself, or its jar archive
+                   :path (case protocol
+                           "file" (.getPath resource-url)
+                           "jar" (jar-path resource-url))
+                   ;; is the resource inside an archive
+                   :archive? (= "jar" protocol)))
           (finally (.close (.getJavaFileManager root))))))
     (catch Throwable _)))

test/orchard/java/DummyClass.java

@@ -0,0 +1,21 @@
+package orchard.java;
+
+/**
+ * Class level docstring.
+ *
+ * <pre>
+ *   DummyClass dc = new DummyClass();
+ * </pre>
+ *
+ * @author Arne Brasseur
+ */
+public class DummyClass {
+  /**
+   * Method-level docstring.
+   *
+   * @returns the string "hello"
+   */
+  public String dummyMethod() {
+    return "hello";
+  }
+}

test/orchard/java/parser_test.clj

@@ -0,0 +1,67 @@
+(ns orchard.java.parser-test
+  (:require [orchard.misc :as misc]
+            [clojure.test :refer :all]))
+
+(when (>= orchard.misc/java-api-version 9)
+  (require '[orchard.java.parser :as parser])
+
+  (defn compile-class-from-source
+    "Compile a java file on the classpath.
+  Returns true if all went well."
+    [classname]
+    (let [compiler (javax.tools.ToolProvider/getSystemJavaCompiler)]
+      (.. compiler
+          (getTask
+           nil ;; out
+           nil ;; fileManager
+           nil ;; diagnosticListener
+           nil ;; compilerOptions
+           nil ;; classnames for annotation processing
+           ;; compilationUnits
+           [(.. compiler
+                (getStandardFileManager nil nil nil)
+                (getJavaFileForInput javax.tools.StandardLocation/CLASS_PATH
+                                     classname
+                                     javax.tools.JavaFileObject$Kind/SOURCE))])
+          call)))
+
+  (deftest source-info-test
+    (is (compile-class-from-source "orchard.java.DummyClass"))
+
+    (testing "file on the filesystem"
+      (is (= {:class 'orchard.java.DummyClass,
+              :members
+              '{orchard.java.DummyClass
+                {[]
+                 {:name orchard.java.DummyClass,
+                  :type void,
+                  :argtypes [],
+                  :argnames [],
+                  :doc nil,
+                  :line 12,
+                  :column 8}},
+                dummyMethod
+                {[]
+                 {:name dummyMethod,
+                  :type java.lang.String,
+                  :argtypes [],
+                  :argnames [],
+                  :doc "Method-level docstring. @returns the string \"hello\"",
+                  :line 18,
+                  :column 3}}},
+              :doc
+              "Class level docstring.\n\n```\n   DummyClass dc = new DummyClass();\n```\n\n@author Arne Brasseur",
+              :line 12,
+              :column 1,
+              :file "orchard/java/DummyClass.java",
+              :path (str (System/getProperty "user.dir")
+                         "/test/orchard/java/DummyClass.java")
+              :archive? false}
+             ((resolve 'orchard.java.parser/source-info) 'orchard.java.DummyClass))))
+
+    (testing "java file in a jar"
+      (let [rt-info ((resolve 'orchard.java.parser/source-info) 'clojure.lang.RT)]
+        (is (= {:file "clojure/lang/RT.java" :archive? true}
+               (select-keys rt-info [:file :archive?])))
+        (is (re-find #"/.*/.m2/repository/org/clojure/clojure/.*/clojure-.*-sources.jar"
+                     (:path rt-info)))))))

test/orchard/java_test.clj

@@ -81,14 +81,16 @@
 (deftest map-structure-test
   (when jdk-parser?
     (testing "Parsed map structure = reflected map structure"
-      (let [cols #{:file :line :column :doc :argnames :argtypes :path}
+      (let [cols #{:file :line :column :doc :argnames :argtypes :path :archive?}
             keys= #(= (set (keys (apply dissoc %1 cols)))
                       (set (keys %2)))
             c1 (class-info* 'clojure.lang.Compiler)
             c2 (with-redefs [source-info (constantly nil)]
                  (class-info* 'clojure.lang.Compiler))]
         ;; Class info
-        (is (keys= c1 c2))
+        (is (keys= c1 c2) (str "Difference: "
+                               (pr-str [(remove (set (keys c1)) (keys c2))
+                                        (remove (set (keys c2)) (keys c1))])))
         ;; Members
         (is (keys (:members c1)))
         (is (= (keys (:members c1))