Skip to content

Commit

Permalink
Dep expansion changes - memoize dep lookups, reenqueue same-version c…
Browse files Browse the repository at this point in the history 
…hildren
  • Loading branch information
@puredanger
puredanger committed Feb 6, 2025
1 parent a46155b commit 57c1d63
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 45 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ Changelog

*Also see [Tools and installer changelog](https://github.com/clojure/brew-install/blob/1.12.0/CHANGELOG.md)*

* next
* Modify dep expansion to reenqueue same version children in case previous parent was omitted
* Memoize dep lookups during dep expansion to avoid unnecessary calls
* 0.21.1471 on Jan 27, 2024
* Fix bug with -Srepro -Spom
* 0.21.1467 on Dec 31, 2024
Expand Down
91 changes: 48 additions & 43 deletions src/main/clojure/clojure/tools/deps.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
[java.io File InputStreamReader BufferedReader]
[java.lang ProcessBuilder ProcessBuilder$Redirect]
[java.util List]
[java.util.concurrent ExecutorService]))
[java.util.concurrent ConcurrentHashMap ExecutorService]))

;(set! *warn-on-reflection* true)

Expand Down Expand Up @@ -227,17 +227,16 @@
:cut' (assoc cut [lib coord-id] coord-excl)
:child-pred (fn [lib] (not (contains? coord-excl lib)))})

;; if seeing same lib/ver again, narrow exclusions to intersection of prior and new.
;; only include new unexcluded children (old excl set minus new excl set)
;; as others were already enqueued when first added
;; if seeing same lib/ver again, narrow exclusions to intersection of prior and new,
;; must reconsider previously included children as prev parent may get omitted
(= reason :same-version)
(let [exclusions' (if (seq coord-excl) (assoc exclusions use-path coord-excl) exclusions)
cut-coord (get cut [lib coord-id]) ;; previously cut from this lib, so were not enqueued
new-cut (set/intersection coord-excl cut-coord)
enq-only (set/difference cut-coord new-cut)]
{:exclusions' exclusions'
:cut' (assoc cut [lib coord-id] new-cut)
:child-pred (set enq-only)})
:child-pred (fn [lib] (not (contains? new-cut lib)))})

:else ;; otherwise, no change
{:exclusions' exclusions, :cut' cut})))
Expand Down Expand Up @@ -394,44 +393,50 @@
(defn- expand-deps
"Dep tree expansion, returns version map"
[deps default-deps override-deps config executor trace?]
(letfn [(err-handler [throwable]
(do
(concurrent/shutdown-on-error executor)
(throw ^Throwable throwable)))
(children-task [lib use-coord use-path child-pred]
{:pend-children
(let [{:deps/keys [manifest root]} use-coord]
(dir/with-dir (if root (jio/file root) dir/*the-dir*)
(concurrent/submit-task executor
#(try
(canonicalize-deps (ext/coord-deps lib use-coord manifest config) config)
(catch Throwable t t)))))
:ppath use-path
:child-pred child-pred})]
(loop [pendq nil ;; a resolved child-lookup thunk to look at first
q (into PersistentQueue/EMPTY (map vector deps)) ;; queue of nodes or child-lookups
version-map nil ;; track all seen versions of libs and which version is selected
exclusions nil ;; tracks exclusions marked in the tree
cut nil ;; tracks cuts made of child nodes based on exclusions
trace []] ;; trace expansion
(let [{:keys [path pendq q']} (next-path pendq q err-handler)]
(if path
(let [[lib coord] (peek path)
parents (pop path)
use-path (conj parents lib)
override-coord (get override-deps lib)
choose-coord (cond override-coord override-coord
coord coord
:else (get default-deps lib))
use-coord (merge choose-coord (ext/manifest-type lib choose-coord config))
coord-id (ext/dep-id lib use-coord config)
{:keys [include reason vmap]} (include-coord? version-map lib use-coord coord-id parents exclusions config)
;_ (println "loop" lib coord-id "include=" include "reason=" reason)
{:keys [exclusions' cut' child-pred]} (update-excl lib use-coord coord-id use-path include reason exclusions cut)
new-q (if child-pred (conj q' (children-task lib use-coord use-path child-pred)) q')]
(recur pendq new-q vmap exclusions' cut'
(trace+ trace? trace parents lib coord use-coord coord-id override-coord include reason)))
(cond-> version-map trace? (with-meta {:trace {:log trace, :vmap version-map, :exclusions exclusions}})))))))
(let [memoized-deps (ConcurrentHashMap. 100)]
(letfn [(err-handler [throwable]
(do
(concurrent/shutdown-on-error executor)
(throw ^Throwable throwable)))
(children-task [lib use-coord use-path child-pred]
{:pend-children
(let [{:deps/keys [manifest root]} use-coord]
(dir/with-dir (if root (jio/file root) dir/*the-dir*)
(concurrent/submit-task executor
#(try
(let [k [lib use-coord]]
(or
(.get memoized-deps k)
(let [child-deps (canonicalize-deps (ext/coord-deps lib use-coord manifest config) config)]
(.putIfAbsent memoized-deps k child-deps)
child-deps)))
(catch Throwable t t)))))
:ppath use-path
:child-pred child-pred})]
(loop [pendq nil ;; a resolved child-lookup thunk to look at first
q (into PersistentQueue/EMPTY (map vector deps)) ;; queue of nodes or child-lookups
version-map nil ;; track all seen versions of libs and which version is selected
exclusions nil ;; tracks exclusions marked in the tree
cut nil ;; tracks cuts made of child nodes based on exclusions
trace []] ;; trace expansion
(let [{:keys [path pendq q']} (next-path pendq q err-handler)]
(if path
(let [[lib coord] (peek path)
parents (pop path)
use-path (conj parents lib)
override-coord (get override-deps lib)
choose-coord (cond override-coord override-coord
coord coord
:else (get default-deps lib))
use-coord (merge choose-coord (ext/manifest-type lib choose-coord config))
coord-id (ext/dep-id lib use-coord config)
{:keys [include reason vmap]} (include-coord? version-map lib use-coord coord-id parents exclusions config)
; _ (println "loop" lib coord-id "include=" include "reason=" reason)
{:keys [exclusions' cut' child-pred]} (update-excl lib use-coord coord-id use-path include reason exclusions cut)
new-q (if child-pred (conj q' (children-task lib use-coord use-path child-pred)) q')]
(recur pendq new-q vmap exclusions' cut'
(trace+ trace? trace parents lib coord use-coord coord-id override-coord include reason)))
(cond-> version-map trace? (with-meta {:trace {:log trace, :vmap version-map, :exclusions exclusions}}))))))))

(defn- cut-orphans
"Remove any selected lib that does not have a selected parent path"
Expand Down
23 changes: 21 additions & 2 deletions src/test/clojure/clojure/tools/deps/test_deps.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,25 @@
'ex/b {:fkn/version "1"}}} nil)]
(libs->lib-ver res))))))

;; +a1 -> -h1 -> -c1 -> -d1
;; +b1 -> +e1 -> +c1 -> +d1
;; -> +h2 -> c1 -> d1
;; h2 supersedes previous h1, but need to ensure d1 is included via c1 somewhere
(deftest test-cut-previously-selected-child-3
(fkn/with-libs {'ex/a {{:fkn/version "1"} [['ex/h {:fkn/version "1"}]]}
'ex/b {{:fkn/version "1"} [['ex/e {:fkn/version "1"}]]}
'ex/c {{:fkn/version "1"} [['ex/d {:fkn/version "1"}]]}
'ex/d {{:fkn/version "1"} nil}
'ex/e {{:fkn/version "1"} [['ex/c {:fkn/version "1"}] ['ex/h {:fkn/version "2"}]]}
'ex/h {{:fkn/version "1"} [['ex/c {:fkn/version "1"}]]
{:fkn/version "2"} [['ex/c {:fkn/version "1"}]]}}
(is (= {:a "1", :b "1", :c "1", :d "1", :e "1", :h "2"}
(let [res (deps/resolve-deps {:deps {'ex/a {:fkn/version "1"}
'ex/b {:fkn/version "1"}}} {:threads 1})]
(libs->lib-ver res))))))

(comment (test-cut-previously-selected-child-3) )

;; +a -> +b -> -x2 -> -y2 -> -z2
;; -> +c -> +d -> +x3 -> +y2 -> +z2
;; -> -x1 -> -y1 -> -z1
Expand Down Expand Up @@ -436,9 +455,9 @@
cut '{[a {:mvn/version "1"}] #{c}}
ret (#'deps/update-excl 'a '{:mvn/version "1" :exclusions [c d]} {:mvn/version "1"} '[b a] false :same-version excl cut)]
(is (= {:exclusions' '{[a] #{c}, [b a] #{c d}}, :cut' cut} (select-keys ret [:exclusions' :cut']))) ;; no change in cut
(let [pred (:child-pred ret)] ;; everything already enqueued
(let [pred (:child-pred ret)] ;; c excluded in both, but re-enqueue d - always intersection
(is (false? (boolean (pred 'c))))
(is (false? (boolean (pred 'd)))))))
(is (true? (boolean (pred 'd)))))))

;; +x1 -> -a1 -> +b2
;; +z1 -> +y1 -> +a2 -> -b1 (or +b1, but at least a consistent result)
Expand Down

0 comments on commit 57c1d63

Please sign in to comment.