diff --git a/CHANGELOG.md b/CHANGELOG.md index 12db15419..c33707314 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ * [#941](https://github.com/clojure-emacs/cider-nrepl/pull/941): Stop vendoring Fipp dependency. * [#941](https://github.com/clojure-emacs/cider-nrepl/pull/941): Default to orchard.pp printer when Fipp/Puget/Zprint is selected but not found on classpath. +* [#943](https://github.com/clojure-emacs/cider-nrepl/pull/943): Debug: reduce insrumentation bytecode footprint. ## 0.55.7 (2025-04-29) diff --git a/project.clj b/project.clj index d0151d6f8..450cd1cf6 100644 --- a/project.clj +++ b/project.clj @@ -52,6 +52,7 @@ :filespecs [{:type :bytes :path "cider/cider-nrepl/project.clj" :bytes ~(slurp "project.clj")}] :source-paths ["src"] + :java-source-paths ["src"] :resource-paths ["resources"] :test-paths ["test/clj" "test/cljs" "test/common"] @@ -137,4 +138,5 @@ :eastwood [:test {:plugins [[jonase/eastwood "1.4.3"]] :eastwood {:config-files ["eastwood.clj"] - :exclude-namespaces [cider.nrepl.middleware.test-filter-tests]}}]}) + :exclude-namespaces [cider.nrepl.middleware.debug-test + cider.nrepl.middleware.test-filter-tests]}}]}) diff --git a/src/cider/nrepl/middleware/DebugSupport.java b/src/cider/nrepl/middleware/DebugSupport.java new file mode 100644 index 000000000..f17ff2206 --- /dev/null +++ b/src/cider/nrepl/middleware/DebugSupport.java @@ -0,0 +1,55 @@ +package cider.nrepl.middleware; + +import clojure.lang.*; + +/** + * Contains instrumentation helpers for cider.nrepl.middleware.debug. The main + * purpose of having these helpers in Java is reducing the instrumentation + * footprint (measured in bytecode size). Invoking Java methods usually takes + * fewer bytecode instructions than a corresponding Clojure function. Java also + * allows us to have primitive overrides which further reduces overhead when + * instrumenting code that contains primitives, but also preserves type hints + * (so instrumented code behaves closer to original code). + * + * The reason we care about bytecode size is 65KB method limit that JVM imposes. + */ +public class DebugSupport { + + private static volatile IFn breakFn = null; + + public static Object doBreak(Object coor, Object val, Object locals, Object STATE__) { + if (breakFn == null) + breakFn = (IFn)RT.var("cider.nrepl.middleware.debug", "break"); + return breakFn.invoke(coor, val, locals, STATE__); + } + + public static long doBreak(Object coor, long val, Object locals, Object STATE__) { + return (long)doBreak(coor, Numbers.num(val), locals, STATE__); + } + + public static double doBreak(Object coor, double val, Object locals, Object STATE__) { + return (double)doBreak(coor, Numbers.num(val), locals, STATE__); + } + + // The purpose of the following assoc methods is to build a locals map. + // Chaining such assoc calls is more bytecode-compact than a single + // RT.mapUniqueKeys(...) because the latter constructs an array (load each + // key and value onto the stack, save them into the array) and boxes + // primitives (invocations of Numbers.num). Additionally, in this custom + // method we turn string keys into symbols, which means we don't have to + // generate symbols at the callsite (in the instrumented method). This saves + // bytecode because LDC of a constant string is more compact than + // ALOAD+GETFIELD of an interned symbol. + + public static IPersistentMap assoc(Object map, Object key, Object value) { + return ((IPersistentMap)map).assoc(Symbol.intern(null, (String)key), value); + } + + public static IPersistentMap assoc(Object map, Object key, long value) { + return assoc(map, key, Numbers.num(value)); + } + + public static IPersistentMap assoc(Object map, Object key, double value) { + return assoc(map, key, Numbers.num(value)); + } +} diff --git a/src/cider/nrepl/middleware/debug.clj b/src/cider/nrepl/middleware/debug.clj index 24ba0edf0..eb956caaa 100644 --- a/src/cider/nrepl/middleware/debug.clj +++ b/src/cider/nrepl/middleware/debug.clj @@ -15,6 +15,7 @@ [orchard.print] [orchard.stacktrace :as stacktrace]) (:import + (cider.nrepl.middleware DebugSupport) (clojure.lang Compiler$LocalBinding) (java.util UUID))) @@ -146,12 +147,13 @@ (remove (fn [[k]] (re-find #"__" (name k))) locals)) ;;; Politely borrowed from clj-debugger. -(defn sanitize-env - "Turn a macro's &env into a map usable for binding." +(defn locals-capturer + "Turn a macro's &env into code that produces a map of locals." [env] - (into {} (for [[sym bind] (filter-env env) - :when (instance? Compiler$LocalBinding bind)] - [`(quote ~sym) (.sym ^Compiler$LocalBinding bind)]))) + `(-> {} + ~@(for [[sym bind] (filter-env env) + :when (instance? Compiler$LocalBinding bind)] + `(DebugSupport/assoc ~(name sym) ~(.sym ^Compiler$LocalBinding bind))))) ;;;; ## Getting user input ;;; @@ -469,7 +471,7 @@ this map (identified by a key), and will `dissoc` it afterwards."} "Let-wrap `body` with STATE__ map containing code, file, line, column etc. STATE__ is an anaphoric variable available to all breakpoint macros. Ends with __ to avid conflicts with user locals and to signify that it's an internal - variable which is cleaned in `sanitize-env' along other clojure's + variable which is cleaned in `locals-capturer` along other clojure's temporaries." {:style/indent 0} [& body] @@ -553,11 +555,11 @@ this map (identified by a key), and will `dissoc` it afterwards."} `(apply-instrumented-maybe (var ~fn-sym) [~@args] ~coor ~'STATE__)) form) locals (when *do-locals* - (sanitize-env &env))] + (locals-capturer &env))] ;; Keep original forms in a separate atom to save some code ;; size. Unfortunately same trick wouldn't work for locals. (swap! *tmp-forms* assoc coor original-form) - `(break ~coor ~val-form ~locals ~'STATE__))) + `(DebugSupport/doBreak ~coor ~val-form ~locals ~'STATE__))) (def irrelevant-return-value-forms "Set of special-forms whose return value we don't care about. @@ -655,8 +657,12 @@ this map (identified by a key), and will `dissoc` it afterwards."} (binding [*tmp-forms* (atom {})] (eval form1)) (catch java.lang.RuntimeException e - (if (re-matches #".*Method code too large!.*" (.getMessage e)) - (do (notify-client (str "Method code too large!\n" + (if (some #(when % + (re-matches #".*Method code too large!.*" + (.getMessage ^Throwable %))) + [e (.getCause e)]) + (do (notify-client *msg* + (str "Method code too large!\n" "Locals and evaluation in local context won't be available.") :warning) ;; re-try without locals diff --git a/src/cider/nrepl/middleware/enlighten.clj b/src/cider/nrepl/middleware/enlighten.clj index a70d628ca..fcae2ee4a 100644 --- a/src/cider/nrepl/middleware/enlighten.clj +++ b/src/cider/nrepl/middleware/enlighten.clj @@ -66,7 +66,7 @@ (symbol? original-form) `(do (send-if-local '~original-form (assoc (:msg ~'STATE__) :coor ~coor) - ~(d/sanitize-env &env)) + ~(d/locals-capturer &env)) ~form) (seq? form) (wrap-function-form form extras) :else form)) diff --git a/test/clj/cider/nrepl/middleware/debug_test.clj b/test/clj/cider/nrepl/middleware/debug_test.clj index a9b12ca10..c017eb36a 100644 --- a/test/clj/cider/nrepl/middleware/debug_test.clj +++ b/test/clj/cider/nrepl/middleware/debug_test.clj @@ -2,8 +2,10 @@ (:require [cider.nrepl.middleware.util.instrument :as ins] [cider.nrepl.middleware.debug :as d] + [cider.test-helpers :refer :all] [clojure.test :refer :all] [clojure.walk :as walk] + [matcher-combinators.matchers :as matchers] [nrepl.middleware.interruptible-eval :refer [*msg*]] [nrepl.transport :as t])) @@ -72,13 +74,13 @@ ~m)) (defmacro locals [] - `~(#'d/sanitize-env &env)) + `~(#'d/locals-capturer &env)) (defmacro add-locals "Add locals to map m." [& [m]] `(assoc ~m - :locals ~(#'d/sanitize-env &env) + :locals ~(#'d/locals-capturer &env) :original-ns "user")) (deftest read-debug-input-roundtrip @@ -200,38 +202,113 @@ (deftest breakpoint ;; Map merging - (with-redefs [d/read-debug-command (fn [_ v _ s] (assoc (:msg s) :value v)) - d/debugger-message (atom [:fake]) - d/*skip-breaks* (atom nil)] - (binding [*msg* {:session (atom {}) - :code :code - :id :id - :file :file - :line :line - :column :column}] - (let [form `(d/with-initial-debug-bindings - (d/breakpoint-if-interesting (inc 10) {:coor [6]} ~'(inc 10))) - m (eval form)] - (are [k v] (= (k m) v) - :value 11 - :file :file - :line :line - :column :column - :code :code - :original-id :id)) - (reset! d/debugger-message [:fake]) - ;; Locals capturing - (is (= (:value (eval `(d/with-initial-debug-bindings - (let [~'x 10] - (d/breakpoint-if-interesting - (locals) - {:coor [1]} nil))))) - '{x 10})) - ;; Top-level sexps are not debugged, just returned. - (is (= (eval `(d/with-initial-debug-bindings - (let [~'x 10] - (d/breakpoint-if-interesting - (locals) - {:coor []} - nil)))) - '{x 10}))))) + (let [capture (atom nil)] + (with-redefs [d/read-debug-command (fn [_ v _ s] + (reset! capture (assoc (:msg s) :value v)) + v) + d/debugger-message (atom [:fake]) + d/*skip-breaks* (atom nil)] + (binding [*msg* {:session (atom {}) + :code :code + :id :id + :file :file + :line :line + :column :column}] + (let [form `(d/with-initial-debug-bindings + (d/breakpoint-if-interesting (inc 10) {:coor [6]} ~'(inc 10))) + m (eval form)] + (is+ {:value 11 + :file :file + :line :line + :column :column + :code :code + :original-id :id} + @capture)) + (reset! d/debugger-message [:fake]) + ;; Locals capturing + (eval `(d/with-initial-debug-bindings + (let [~'x 10] + (d/breakpoint-if-interesting + (locals) {:coor [1]} nil)))) + (is (= (:value @capture) '{x 10})) + ;; Top-level sexps are not debugged, just returned. + (is (= (eval `(d/with-initial-debug-bindings + (let [~'x 10] + (d/breakpoint-if-interesting + (locals) + {:coor []} + nil)))) + '{x 10})))))) + +(deftest instrumentation-stress-test + (testing "able to compile this function full of locals" + (is + (-> '(defn a-fn [a0] + (let [a0 (long (+)) + a1 (long (+ a0)) + a2 (+ a0 a1) + a3 (+ a0 a1 a2) + a4 (+ a0 a1 a2 a3) + a5 (+ a0 a1 a2 a3 a4) + a6 (+ a0 a1 a2 a3 a4 a5) + a7 (+ a0 a1 a2 a3 a4 a5 a6) + a8 (+ a0 a1 a2 a3 a4 a5 a6 a7) + a9 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8) + a10 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9) + a11 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10) + a12 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11) + a13 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12) + a14 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12 a13) + a15 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12 a13 a14) + a16 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12 a13 a14 a15) + a17 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12 a13 a14 a15 a16) + a18 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12 a13 a14 a15 a16 a17) + a19 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12 a13 a14 a15 a16 a17 a18) + a20 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12 a13 a14 a15 a16 a17 a18 a19) + a21 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12 a13 a14 a15 a16 a17 a18 a19 a20) + a22 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12 a13 a14 a15 a16 a17 a18 a19 a20 a21) + a23 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12 a13 a14 a15 a16 a17 a18 a19 a20 a21 a22) + a24 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12 a13 a14 a15 a16 a17 a18 a19 a20 a21 a22 a23) + a25 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12 a13 a14 a15 a16 a17 a18 a19 a20 a21 a22 a23 a24) + a26 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12 a13 a14 a15 a16 a17 a18 a19 a20 a21 a22 a23 a24 a25)] + a0)) + d/debug-reader + ins/instrument-tagged-code + eval))) + + (testing "fails if there is one extra line" + (is (thrown? clojure.lang.Compiler$CompilerException + (-> '(defn a-fn [a0] + (let [a0 (long (+)) + a1 (long (+ a0)) + a2 (+ a0 a1) + a3 (+ a0 a1 a2) + a4 (+ a0 a1 a2 a3) + a5 (+ a0 a1 a2 a3 a4) + a6 (+ a0 a1 a2 a3 a4 a5) + a7 (+ a0 a1 a2 a3 a4 a5 a6) + a8 (+ a0 a1 a2 a3 a4 a5 a6 a7) + a9 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8) + a10 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9) + a11 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10) + a12 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11) + a13 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12) + a14 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12 a13) + a15 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12 a13 a14) + a16 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12 a13 a14 a15) + a17 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12 a13 a14 a15 a16) + a18 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12 a13 a14 a15 a16 a17) + a19 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12 a13 a14 a15 a16 a17 a18) + a20 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12 a13 a14 a15 a16 a17 a18 a19) + a21 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12 a13 a14 a15 a16 a17 a18 a19 a20) + a22 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12 a13 a14 a15 a16 a17 a18 a19 a20 a21) + a23 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12 a13 a14 a15 a16 a17 a18 a19 a20 a21 a22) + a24 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12 a13 a14 a15 a16 a17 a18 a19 a20 a21 a22 a23) + a25 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12 a13 a14 a15 a16 a17 a18 a19 a20 a21 a22 a23 a24) + a26 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12 a13 a14 a15 a16 a17 a18 a19 a20 a21 a22 a23 a24 a25) + a27 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12 a13 a14 a15 a16 a17 a18 a19 a20 a21 a22 a23 a24 a25 a26) + a28 (+ a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 a10 a11 a12 a13 a14 a15 a16 a17 a18 a19 a20 a21 a22 a23 a24 a25 a26 a27)] + a0)) + d/debug-reader + ins/instrument-tagged-code + eval)))))