Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve show performance by only evaluating each :render-fn once #495

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 16 additions & 27 deletions src/nextjournal/clerk/render.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -217,13 +217,16 @@
(defn read-string [s]
(js/nextjournal.clerk.sci_env.read-string s))

(defn replace-viewer-fns [{:as doc :keys [name->viewer]}]
(assoc (w/postwalk-replace name->viewer doc)
:name->viewer name->viewer))

(defn fetch! [{:keys [blob-id]} opts]
#_(js/console.log :fetch! blob-id opts)
(-> (js/fetch (str "/_blob/" blob-id (when (seq opts)
(str "?" (opts->query opts)))))
(.then #(.text %))
(.then #(try (read-string %)
(.then #(try (replace-viewer-fns (read-string %))
(catch js/Error e
(js/console.error #js {:message "sci read error" :blob-id blob-id :code-string % :error e})
(render-unreadable-edn %))))))
Expand Down Expand Up @@ -345,21 +348,20 @@
[triangle expanded?]]
[:span.group-hover:text-indigo-700 opening-paren]]))

(defn render-coll [xs {:as opts :keys [path viewer !expanded-at] :or {path []}}]
(defn render-coll [xs {:as opts :keys [closing-parens path viewer !expanded-at] :or {path []}}]
(let [expanded? (get @!expanded-at path)
{:keys [opening-paren closing-paren]} viewer]
[:span.inspected-value.whitespace-nowrap
{:class (when expanded? "inline-flex")}
[:span
(if (< 1 (count xs))
(if (expandable? xs)
[expand-button !expanded-at opening-paren path]
[:span opening-paren])
(into [:<>]
(comp (inspect-children opts)
(interpose (if expanded? [:<> [:br] triangle-spacer nbsp (when (= 2 (count opening-paren)) nbsp)] " ")))
xs)
[:span
(cond->> closing-paren (list? closing-paren) (into [:<>]))]]]))
(into [:span] (or closing-parens [closing-paren]))]]))

(defn render-elision [{:as fetch-opts :keys [total offset unbounded?]} _]
[view-context/consume :fetch-fn
Expand All @@ -372,21 +374,6 @@
:on-click #(when (fn? fetch-fn)
(fetch-fn fetch-opts))} (- total offset) (when unbounded? "+") (if (fn? fetch-fn) " more…" " more elided")])])

(defn render-map [xs {:as opts :keys [path viewer !expanded-at] :or {path []}}]
(let [expanded? (get @!expanded-at path)
{:keys [closing-paren]} viewer]
[:span.inspected-value.whitespace-nowrap
{:class (when expanded? "inline-flex")}
[:span
(if (expandable? xs)
[expand-button !expanded-at "{" path]
[:span "{"])
(into [:<>]
(comp (inspect-children opts)
(interpose (if expanded? [:<> [:br] triangle-spacer nbsp #_(repeat (inc (count path)) nbsp)] " ")))
xs)
(cond->> closing-paren (list? closing-paren) (into [:<>]))]]))


(defn render-string [s {:as opts :keys [path !expanded-at] :or {path []}}]
(let [expanded? (get @!expanded-at path)]
Expand All @@ -398,16 +385,16 @@
(inspect-presented opts %)))
(if (string? s) [s] s))))

(defn render-quoted-string [s {:as opts :keys [path viewer !expanded-at] :or {path []}}]
(defn render-quoted-string [s {:as opts :keys [closing-parens path viewer !expanded-at] :or {path []}}]
(let [{:keys [opening-paren closing-paren]} viewer]
[:span.inspected-value.inline-flex
[:span.cmt-string
(if (some #(and (string? %) (str/includes? % "\n")) (if (string? s) [s] s))
[expand-button !expanded-at opening-paren path]
[:span opening-paren])]
[:div
[:span.cmt-string (viewer/->value (render-string s opts)) (first closing-paren)]
(when (list? closing-paren) (into [:<>] (rest closing-paren)))]]))
(into [:div
[:span.cmt-string (viewer/->value (render-string s opts)) (first closing-paren)]
(rest closing-parens)])]))

(defn render-number [num]
[:span.cmt-number.inspected-value
Expand Down Expand Up @@ -528,6 +515,8 @@
(if (valid-react-element? x)
x
(let [{:nextjournal/keys [value viewer] :keys [path]} x]
(when-not (:render-fn viewer)
(throw (ex-info "A render function is missing" {:viewer viewer})))
#_(prn :inspect-presented value :valid-element? (react/isValidElement value) :viewer viewer)
;; each view function must be called in its own 'functional component' so that it gets its own hook state.
^{:key (str (:hash viewer) "@" (peek (:path opts)))}
Expand Down Expand Up @@ -631,7 +620,7 @@
(when (exists? js/window)
;; TODO: can we restore the scroll position when navigating back?
(.scrollTo js/window #js {:top 0}))
(reset! !doc doc))
(reset! !doc (replace-viewer-fns doc)))
;; (when (and error (contains? @!doc :status))
;; (swap! !doc dissoc :status))
(when (remount? doc)
Expand All @@ -644,10 +633,10 @@

(defn patch-state! [{:keys [patch]}]
(if (remount? patch)
(do (swap! !doc #(re-eval-viewer-fns (apply-patch % patch)))
(do (swap! !doc #(re-eval-viewer-fns (replace-viewer-fns (apply-patch % patch))))
;; TODO: figure out why it doesn't work without `js/setTimeout`
(js/setTimeout #(swap! !eval-counter inc) 10))
(swap! !doc apply-patch patch)))
(swap! !doc #(replace-viewer-fns (apply-patch % patch)))))

(defonce !pending-clerk-eval-replies
(atom {}))
Expand Down
24 changes: 20 additions & 4 deletions src/nextjournal/clerk/view.clj
Original file line number Diff line number Diff line change
@@ -1,15 +1,31 @@
(ns nextjournal.clerk.view
(:require [nextjournal.clerk.viewer :as v]
[hiccup.page :as hiccup]
(:require [clojure.java.io :as io]
[clojure.set :as set]
[clojure.string :as str]
[clojure.java.io :as io])
[clojure.walk :as walk]
[hiccup.page :as hiccup]
[nextjournal.clerk.viewer :as v])
(:import (java.net URI)))


(defn ^:private extract-name->viewer [presentation]
(into {}
(map (juxt (fn [viewer]
(or (when (qualified-symbol? (:name viewer))
(symbol (namespace (:name viewer)) (str (name (:name viewer)) "$" (:hash viewer))))
(symbol "nextjournal.clerk.viewer" (str "viewer-fn$" (:hash viewer))))) identity))
(keep :nextjournal/viewer (tree-seq (some-fn map? vector?) #(cond-> % (map? %) vals) presentation))))

(defn +name->viewer [presentation]
(let [name->viewer (extract-name->viewer presentation)]
(assoc (walk/postwalk-replace (set/map-invert name->viewer) presentation)
:name->viewer name->viewer)))

(defn doc->viewer
([doc] (doc->viewer {} doc))
([opts {:as doc :keys [ns file]}]
(binding [*ns* ns]
(-> (merge doc opts) v/notebook v/present))))
(-> (merge doc opts) v/notebook v/present +name->viewer))))

#_(doc->viewer (nextjournal.clerk/eval-file "notebooks/hello.clj"))
#_(nextjournal.clerk/show! "notebooks/test.clj")
Expand Down
11 changes: 5 additions & 6 deletions src/nextjournal/clerk/viewer.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -838,7 +838,7 @@
{:name `sequential-viewer :pred sequential? :render-fn 'nextjournal.clerk.render/render-coll :opening-paren "(" :closing-paren ")" :page-size 20})

(def map-viewer
{:name `map-viewer :pred map? :render-fn 'nextjournal.clerk.render/render-map :opening-paren "{" :closing-paren "}" :page-size 10})
{:name `map-viewer :pred map? :render-fn 'nextjournal.clerk.render/render-coll :opening-paren "{" :closing-paren "}" :page-size 10})

#?(:cljs (defn var->symbol [v] (if (instance? sci.lang.Var v) (sci.impl.vars/toSymbol v) (symbol v))))

Expand Down Expand Up @@ -1548,7 +1548,7 @@

(defn ^:private present-elision* [!path->wrapped-value {:as fetch-opts :keys [path]}]
(if-let [wrapped-value (@!path->wrapped-value path)]
(present* (merge wrapped-value (make-!budget-opts wrapped-value) fetch-opts))
(assign-closing-parens (present* (merge wrapped-value (make-!budget-opts wrapped-value) fetch-opts)))
(throw (ex-info "could not find wrapped-value at path" {:!path->wrapped-value !path->wrapped-value :fetch-otps fetch-opts}))))


Expand Down Expand Up @@ -1722,10 +1722,9 @@
(or (-> value last :nextjournal/viewer :closing-paren) ;; the last element can carry parens
(and (= `map-entry-viewer (-> value last :nextjournal/viewer :name)) ;; the last element is a map entry whose value can carry parens
(-> value last :nextjournal/value last :nextjournal/viewer :closing-paren))))]
(cond-> (cond
(not closing) node
defer-closing? (update node :nextjournal/viewer dissoc :closing-paren)
:else (update-in node [:nextjournal/viewer :closing-paren] cons closing-parens))
(cond-> (assoc-in node [:nextjournal/render-opts :closing-parens] (if (or (not closing) defer-closing?)
'()
(cons closing closing-parens)))
non-leaf? (update :nextjournal/value
(fn [xs]
(into []
Expand Down
4 changes: 1 addition & 3 deletions src/nextjournal/clerk/webserver.clj
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@
(if (contains? desc :nextjournal/content-type)
{:body (v/->value desc)
:content-type (:nextjournal/content-type desc)}
{:body (v/->edn desc)}))
{:body (v/->edn (view/+name->viewer desc))}))
{:status 404}))

(defn extract-blob-opts [{:as _req :keys [uri query-string]}]
Expand Down Expand Up @@ -173,8 +173,6 @@
(apply swap! nextjournal.clerk.atom/my-state (eval '[update :counter inc]))
(eval '(nextjournal.clerk/recompute!)))

(declare present+reset!)

(defn ->nav-path [file-or-ns]
(cond (or (symbol? file-or-ns) (instance? clojure.lang.Namespace file-or-ns))
(str "'" file-or-ns)
Expand Down
4 changes: 2 additions & 2 deletions test/nextjournal/clerk/eval_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -173,14 +173,14 @@
(mapv #(select-keys % [:nextjournal/width]))))))

(testing "can handle uncounted sequences"
(is (match? [{:nextjournal/viewer {:name `viewer/code-block-viewer}
(is (match? [{:nextjournal/viewer `viewer/code-block-viewer$5dru1FUcVRTRrVKJFbNw4FG2wXmiwB
:nextjournal/value "(range)"}
{:nextjournal/value {:nextjournal/fetch-opts {:blob-id string?}
:nextjournal/hash string?}}]
(eval+extract-doc-blocks "(range)"))))

(testing "assigns folded visibility"
(is (match? [{:nextjournal/viewer {:name `viewer/folded-code-block-viewer}
(is (match? [{:nextjournal/viewer `viewer/folded-code-block-viewer$5dt3F3pXDCJHWEKwRWd1FTwBTC7bQ1
:nextjournal/value "{:some :map}"}
{:nextjournal/value {:nextjournal/fetch-opts {:blob-id string?}
:nextjournal/hash string?}}]
Expand Down
37 changes: 22 additions & 15 deletions test/nextjournal/clerk/viewer_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -179,18 +179,21 @@
(is (match? {:nextjournal/value "10/33"}
(v/present 10/33))))

(testing "opts are not propagated to children during presentation"
(let [count-opts (fn [o]
(testing "render opts are not propagated to children during presentation"
(let [count-opts (fn [o k]
(let [c (atom 0)]
(w/postwalk (fn [f] (when (= :nextjournal/render-opts f) (swap! c inc)) f) o)
(w/postwalk (fn [f] (when (and (map? f)
(contains? f :nextjournal/render-opts)
(-> f :nextjournal/render-opts k))
(swap! c inc)) f) o)
@c))]
(let [presented (v/present (v/col {:nextjournal.clerk/render-opts {:width 150}} 1 2 3))]
(is (= {:width 150} (:nextjournal/render-opts presented)))
(is (= 1 (count-opts presented))))
(is (match? {:width 150} (:nextjournal/render-opts presented)))
(is (= 1 (count-opts presented :width))))

(let [presented (v/present (v/table {:col1 [1 2] :col2 '[a b]}))]
(is (= {:num-cols 2 :number-col? #{0}} (:nextjournal/render-opts presented)))
(is (= 1 (count-opts presented))))))
(is (match? {:num-cols 2 :number-col? #{0}} (:nextjournal/render-opts presented)))
(is (= 1 (count-opts presented :num-cols))))))

(testing "viewer opts are normalized"
(is (= (v/desc->values (v/present {:nextjournal/value (range 10) :nextjournal/budget 3}))
Expand Down Expand Up @@ -225,14 +228,14 @@
(-> after
(get-in (path-to-value [0 1 1]))
(get 2)
v/->viewer
:closing-paren)))
:nextjournal/render-opts
:closing-parens)))
(is (= '(")" "}")
(-> after
(get-in (path-to-value [1]))
(get 1)
v/->viewer
:closing-paren))))))
:nextjournal/render-opts
:closing-parens))))))

(defn tree-re-find [data re]
(->> data
Expand Down Expand Up @@ -351,23 +354,27 @@
(is (= 5
(count
(->> (eval/eval-string "^{:nextjournal.clerk/budget 5}(reduce (fn [acc _i] (vector acc)) :fin (range 100 0 -1))")
view/doc->viewer v/->value :blocks
view/doc->viewer v/->value
:blocks
(tree-seq coll? seq)
(filter (every-pred map? (comp #{'nextjournal.clerk.render/render-coll} :form :render-fn)))))))
(keep :nextjournal/viewer)
(filter #{'nextjournal.clerk.viewer/vector-viewer$5dsD1KJESfc8Dy8gPeGQfZCX2ayE8f})))))

(is (= 5
(count
(->> (eval/eval-string "(nextjournal.clerk/with-viewer {} {:nextjournal.clerk/budget 5} (reduce (fn [acc i] (vector acc)) :fin (range 15 0 -1)))")
view/doc->viewer v/->value :blocks
(tree-seq coll? seq)
(filter (every-pred map? (comp #{'nextjournal.clerk.render/render-coll} :form :render-fn)))))))
(keep :nextjournal/viewer)
(filter #{'nextjournal.clerk.viewer/vector-viewer$5dsD1KJESfc8Dy8gPeGQfZCX2ayE8f})))))

(is (= 101
(count
(->> (eval/eval-string "^{:nextjournal.clerk/budget nil}(reduce (fn [acc i] (vector i acc)) :fin (range 101 0 -1))")
view/doc->viewer v/->value :blocks
(tree-seq coll? seq)
(filter (every-pred map? (comp #{'nextjournal.clerk.render/render-coll} :form :render-fn)))))))))
(keep :nextjournal/viewer)
(filter #{'nextjournal.clerk.viewer/vector-viewer$5dsD1KJESfc8Dy8gPeGQfZCX2ayE8f})))))))

(deftest ->edn
(testing "normal symbols and keywords"
Expand Down
3 changes: 1 addition & 2 deletions test/nextjournal/clerk/webserver_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
{:nextjournal/keys [value]} presented
{elision-viewer :nextjournal/viewer elision-fetch-opts :nextjournal/value} (peek value)
{:keys [body]} (webserver/serve-blob doc (merge fetch-opts {:fetch-opts elision-fetch-opts}))]
(is (= `nextjournal.clerk.viewer/elision-viewer (:name elision-viewer)))
(is (= `nextjournal.clerk.viewer/elision-viewer$5drduatKq2QJCDhSX1Pu45i4whSPHk elision-viewer))
(is body)
(is (= (-> body webserver/read-msg :nextjournal/value first :nextjournal/value) 20)))))