[debug] Reduce instrumentation footprint

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)
 

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]}}]})

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));
+    }
+}

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

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))

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)))))