Skip to content

Commit

Permalink
CLJS-1228: cljs.util/topo-sort is polynomial on larger dependency graphs
Browse files Browse the repository at this point in the history
remove ns-dependencies, remove corresponding build api. remove tests.

simplify cljs.compiler/requires-compilation? now that we know that the
build order is fixed.
  • Loading branch information
dnolen committed Nov 4, 2015
1 parent 409d1ec commit 9d1b008
Show file tree
Hide file tree
Showing 6 changed files with 8 additions and 100 deletions.
25 changes: 0 additions & 25 deletions src/main/clojure/cljs/analyzer.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -782,31 +782,6 @@
(when ns
(get-in namespaces [ns :macros sym]))))))

(defn ns-dependents
"Given a namespace as a symbol and a map from namespace symbol to
namespace information return the topologically sorted list of all
dependent namespaces. The map values of the optional second argument must
be maps with a :requires set of symbols, a :requires map of symbol -> alias
(analyzer format) or a :requires vector of munged namespace strings
(closure format)."
([ns] (ns-dependents ns (get @env/*compiler* ::namespaces)))
([ns ns-map]
(letfn [(parent? [parent [child {:keys [requires] :as ns-info}]]
(when-not (= parent child)
(cond
(or (map? requires)
(set? requires)) (contains? requires parent)
(vector? requires) (some #{(munge (name parent))} requires))))]
(topo-sort ns
(fn [ns']
(set (map first (filter #(parent? ns' %) ns-map))))))))

(comment
(ns-dependents 'bar
'{bar {:requires #{cljs.core}}
foo {:requires #{cljs.core bar}}})
)

(declare analyze analyze-symbol analyze-seq)

(def specials '#{if def fn* do let* loop* letfn* throw try recur new set!
Expand Down
14 changes: 0 additions & 14 deletions src/main/clojure/cljs/build/api.clj
Original file line number Diff line number Diff line change
Expand Up @@ -70,20 +70,6 @@
(intersection namespaces-set (-> x :require-macros vals set))))
(vals (:cljs.analyzer/namespaces @state)))))))

(defn cljs-ns-dependents
"Given a namespace symbol return a seq of all dependent
namespaces sorted in dependency order. Will include
transient dependents."
([ns]
(cljs-ns-dependents
(if-not (nil? env/*compiler*)
env/*compiler*
(env/default-compiler-env))
ns))
([state ns]
(env/with-compiler-env state
(ana/ns-dependents ns))))

(defn parse-js-ns
"Given a Google Closure style JavaScript file or resource return the namespace
information for the given file. Only returns the value extracted from the
Expand Down
4 changes: 2 additions & 2 deletions src/main/clojure/cljs/closure.clj
Original file line number Diff line number Diff line change
Expand Up @@ -1769,8 +1769,8 @@
(assoc :js-dependency-index (deps/js-dependency-index all-opts))
;; Save list of sources for cljs.analyzer/locate-src - Juho Teperi
(assoc :sources (-find-sources source all-opts))))
(binding [comp/*dependents* (when-not (false? (:recompile-dependents opts))
(atom {:recompile #{} :visited #{}}))
(binding [comp/*recompiled* (when-not (false? (:recompile-dependents opts))
(atom #{}))
ana/*cljs-static-fns*
(or (and (= (:optimizations opts) :advanced)
(not (false? (:static-fns opts))))
Expand Down
19 changes: 6 additions & 13 deletions src/main/clojure/cljs/compiler.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@

(def js-reserved ana/js-reserved)

(def ^:dynamic *dependents* nil)
(def ^:dynamic *recompiled* nil)
(def ^:dynamic *inputs* nil)
(def ^:dynamic *source-map-data* nil)
(def ^:dynamic *lexical-renames* {})
Expand Down Expand Up @@ -1268,7 +1268,7 @@
"Return true if the src file requires compilation."
([src dest] (requires-compilation? src dest nil))
([^File src ^File dest opts]
(let [{:keys [ns]} (ana/parse-ns src)]
(let [{:keys [ns requires]} (ana/parse-ns src)]
(ensure
(or (not (.exists dest))
(> (.lastModified src) (.lastModified dest))
Expand All @@ -1283,10 +1283,8 @@
(if (= (:optimizations opts) :none)
(not (.exists (io/file (str (.getPath dest) ".map"))))
(not (get-in @env/*compiler* [::compiled-cljs (.getAbsolutePath dest)]))))
(let [{:keys [recompile visited]}
(and *dependents* @*dependents*)]
(and (contains? recompile ns)
(not (contains? visited ns))))))))))
(when-let [recompiled' (and *recompiled* @*recompiled*)]
(some requires recompiled'))))))))

#?(:clj
(defn compile-file
Expand Down Expand Up @@ -1331,13 +1329,8 @@
(not= :interactive (:mode opts)))
(swap! env/*compiler* update-in [::ana/namespaces] dissoc ns))
(let [ret (compile-file* src-file dest-file opts)]
(when *dependents*
(swap! *dependents*
(fn [{:keys [recompile visited]}]
{:recompile (into recompile
(ana/ns-dependents ns
(merge *inputs* nses)))
:visited (conj visited ns)})))
(when *recompiled*
(swap! *recompiled* conj ns))
ret))
(do
;; populate compilation environment with analysis information
Expand Down
33 changes: 0 additions & 33 deletions src/test/clojure/cljs/analyzer_tests.clj
Original file line number Diff line number Diff line change
Expand Up @@ -305,39 +305,6 @@
(is (= (meta (:name (a/analyze ns-env '(ns ^{:foo bar} weeble {:foo baz}))))
{:foo 'baz}))))

(def test-deps-env (atom
{:cljs.analyzer/namespaces
{'file-reloading {:requires {'utils 'utils}}
'core {:requires {'file-reloading 'file-reloading}}
'dev {:requires {'client 'client
'core 'core}}
'client {:requires {'utils 'utils}}
'utils {:requires {}}}}))

(deftest test-ns-dependents
(binding [env/*compiler* test-deps-env]
(is (= (set (a/ns-dependents 'client))
#{'dev}))
(is (= (set (a/ns-dependents 'core))
#{'dev}))
(is (= (set (a/ns-dependents 'dev))
#{}))

;; if 'file-reloading returns 'core
(is (= (set (a/ns-dependents 'file-reloading))
#{'dev 'core}))

;; how can 'utils include 'file-reloading but not 'core
;; FAILS with
;; actual: (not (= #{file-reloading dev client} #{file-reloading dev client core}))
(is (= (set (a/ns-dependents 'utils))
#{'file-reloading 'dev 'client 'core}))))

(deftest test-ns-dependents-custom-ns-map
(let [ns-map '{bar {:requires #{cljs.core}}
foo {:requires #{cljs.core bar}}}]
(is (= (set (a/ns-dependents 'bar ns-map)) #{'foo}))))

(deftest test-cljs-1105
;; munge turns - into _, must preserve the dash first
(is (not= (a/gen-constant-id :test-kw)
Expand Down
13 changes: 0 additions & 13 deletions src/test/clojure/cljs/build_api_tests.clj
Original file line number Diff line number Diff line change
Expand Up @@ -104,19 +104,6 @@
(:require [graph.foo.core :as foo]
[graph.bar.core :as bar]))))))

(deftest test-cljs-ns-dependencies
(is (= (env/with-compiler-env test-cenv
(cljs-ns-dependents 'clojure.string))
'(cljs.user)))
(is (= (env/with-compiler-env test-cenv
(cljs-ns-dependents 'foo.core))
'(bar.core baz.core)))
(is (= (env/with-compiler-env test-cenv
(cljs-ns-dependents 'graph.foo.core))
'(graph.bar.core graph.baz.core)))
(is (= (cljs-ns-dependents test-cenv 'graph.foo.core)
'(graph.bar.core graph.baz.core))))

(deftest cljs-1469
(let [srcs "samples/hello/src"
[common-tmp app-tmp] (mapv #(File/createTempFile % ".js")
Expand Down

0 comments on commit 9d1b008

Please sign in to comment.