Skip to content

Commit

Permalink
dance around DCE issues (#37)
Browse files Browse the repository at this point in the history
  • Loading branch information
darwin committed Apr 12, 2017
1 parent adbb78c commit 30a3497
Show file tree
Hide file tree
Showing 13 changed files with 550 additions and 446 deletions.
2 changes: 2 additions & 0 deletions project.clj
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,12 @@
["test-tests"]
["test-tests-with-config"]
["test-dead-code"]
["test-dce-size"]
["test-advanced-warning"]]
"test-dead-code" ["do"
["with-profile" "+testing" "cljsbuild" "once" "dead-code"]
["shell" "test/scripts/dead-code-check.sh"]]
"test-dce-size" ["shell" "scripts/check-dce-size.sh" "+testing"]
"compare-dead-code" ["shell" "scripts/compare-dead-code.sh" "+testing"]
"compare-dead-code-with-pseudo-names" ["shell" "scripts/compare-dead-code.sh" "+testing,+dce-pseudo-names"]
"test-tests" ["do"
Expand Down
60 changes: 60 additions & 0 deletions scripts/check-dce-size.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
#!/usr/bin/env bash

set -e

pushd `dirname "${BASH_SOURCE[0]}"` > /dev/null
source "./config.sh"

cd "$ROOT"

if [[ -z "${SKIP_DCE_COMPILATION}" ]]; then
./scripts/compile-dead-code.sh "$@"
fi

cd "$DCE_CACHE_DIR"

NO_DEBUG_MAX_SIZE=6000
NO_REQUIRE_MAX_SIZE=6000

BUILD_WITH_DEBUG="$DCE_CACHE_DIR/with-debug.js"
BUILD_NO_DEBUG="$DCE_CACHE_DIR/no-debug.js"
BUILD_NO_MENTION="$DCE_CACHE_DIR/no-mention.js"
BUILD_NO_REQUIRE="$DCE_CACHE_DIR/no-require.js"
BUILD_NO_SOURCES="$DCE_CACHE_DIR/no-sources.js"

WITH_DEBUG_SIZE=$(stat -f %z "$BUILD_WITH_DEBUG")
NO_DEBUG_SIZE=$(stat -f %z "$BUILD_NO_DEBUG")
NO_MENTION_SIZE=$(stat -f %z "$BUILD_NO_MENTION")
NO_REQUIRE_SIZE=$(stat -f %z "$BUILD_NO_REQUIRE")
NO_SOURCES_SIZE=$(stat -f %z "$BUILD_NO_SOURCES")

echo
echo "stats:"
echo "WITH_DEBUG: $WITH_DEBUG_SIZE bytes"
echo "NO_DEBUG: $NO_DEBUG_SIZE bytes"
echo "NO_MENTION: $NO_MENTION_SIZE bytes"
echo "NO_REQUIRE: $NO_REQUIRE_SIZE bytes"
echo "NO_SOURCES: $NO_SOURCES_SIZE bytes"
echo

if [[ "$NO_DEBUG_SIZE" != "$NO_MENTION_SIZE" ]]; then
echo "failure: NO_DEBUG and NO_MENTION expected to have the same size."
exit 1
fi

if [[ "$NO_REQUIRE_SIZE" != "$NO_SOURCES_SIZE" ]]; then
echo "failure: NO_REQUIRE and NO_SOURCES expected to have the same size."
exit 2
fi

if [[ "$NO_DEBUG_SIZE" -gt "$NO_DEBUG_MAX_SIZE" ]]; then
echo "failure: NO_DEBUG expected to have size smaller than $NO_DEBUG_MAX_SIZE bytes."
exit 3
fi

if [[ "$NO_REQUIRE_SIZE" -gt "$NO_REQUIRE_MAX_SIZE" ]]; then
echo "failure: NO_REQUIRE_SIZE expected to have size smaller than $NO_REQUIRE_MAX_SIZE bytes."
exit 4
fi

popd
8 changes: 4 additions & 4 deletions src/lib/devtools/core.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
(defn available?
([] (available? (prefs/pref :features-to-install)))
([features-desc]
(let [features (resolve-features! features-desc feature-groups)]
(let [features (resolve-features! features-desc @feature-groups)]
(if (empty? features)
false
(every? is-feature-available? features)))))
Expand All @@ -34,7 +34,7 @@
(defn installed?
([] (installed? (prefs/pref :features-to-install)))
([features-desc]
(let [features (resolve-features! features-desc feature-groups)]
(let [features (resolve-features! features-desc @feature-groups)]
(if (empty? features)
false
(every? is-feature-installed? features)))))
Expand All @@ -44,8 +44,8 @@
([features-desc]
(if (under-advanced-build?)
(display-advanced-build-warning-if-needed!)
(let [features (resolve-features! features-desc feature-groups)]
(display-banner-if-needed! features feature-groups)
(let [features (resolve-features! features-desc @feature-groups)]
(display-banner-if-needed! features @feature-groups)
(print-config-overrides-if-requested! "config overrides prior install:\n")
(install-feature! :formatters features is-feature-available? formatters/install!)
(install-feature! :hints features is-feature-available? hints/install!)
Expand Down
783 changes: 392 additions & 391 deletions src/lib/devtools/defaults.cljs

Large diffs are not rendered by default.

10 changes: 2 additions & 8 deletions src/lib/devtools/formatters/templating.cljs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
(ns devtools.formatters.templating
(:require [cljs.pprint]
[clojure.walk :refer [prewalk]]
[devtools.util :refer-macros [oget oset ocall oapply safe-call]]
(:require [clojure.walk :refer [prewalk]]
[devtools.util :refer-macros [oget oset ocall oapply safe-call] :refer [pprint-str]]
[devtools.protocols :refer [ITemplate IGroup ISurrogate IFormat]]
[devtools.formatters.helpers :refer [pref cljs-value?]]
[devtools.formatters.state :refer [get-current-state prevent-recursion?]]
Expand Down Expand Up @@ -118,11 +117,6 @@
(def ^:dynamic *current-render-stack* [])
(def ^:dynamic *current-render-path* [])

(defn pprint-str [markup]
(with-out-str
(binding [*print-level* 300]
(cljs.pprint/pprint markup))))

(defn print-preview [markup]
(binding [*print-level* 1]
(pr-str markup)))
Expand Down
14 changes: 12 additions & 2 deletions src/lib/devtools/hints.cljs
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
(ns devtools.hints
(:require-macros [devtools.util :refer [emit-if-compiler-in-dev-mode]])
(:require [devtools.prefs :refer [pref]]
[devtools.context :as context]
[cljs.stacktrace :as stacktrace]))
[goog.string] ; for cljs.stacktrace, see devtools.optional
[clojure.string])) ; for cljs.stacktrace, see devtools.optional

(emit-if-compiler-in-dev-mode (goog/require "devtools.optional"))

(defn ^:dynamic available? []
true)
Expand Down Expand Up @@ -92,10 +96,16 @@
(re-matches #"Cannot read property 'call' of.*" message) (mark-null-call-site-location file line-number column)
:else nil))

(defn parse-stacktrace [native-stack-trace]
; note that we have to do dynamic lookup here, cljs.stacktrace is present only in dev mode
; or when explicitly required by the user of cljs-devtools
(if (exists? js/devtools.optional.parse-stacktrace)
(js/devtools.optional.parse-stacktrace {} native-stack-trace {:ua-product :chrome} {:asset-root ""})))

(defn error-object-sense [error]
(try
(let [native-stack-trace (.-stack error)
stack-trace (stacktrace/parse-stacktrace {} native-stack-trace {:ua-product :chrome} {:asset-root ""})
stack-trace (parse-stacktrace native-stack-trace)
top-item (second stack-trace) ; first line is just an error message
{:keys [file line column]} top-item]
(make-sense-of-the-error (.-message error) file line column))
Expand Down
4 changes: 2 additions & 2 deletions src/lib/devtools/munging.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,7 @@
name (last parts)]
[ns name protocol-selector]))

(def fast-path-protocols-lookup-table (get-fast-path-protocols-lookup-table))
(def fast-path-protocols-lookup-table (delay (get-fast-path-protocols-lookup-table)))

(defn key-for-protocol-partition [partition]
(str "cljs$lang$protocol_mask$partition" partition "$"))
Expand All @@ -556,7 +556,7 @@
(let [partition-key (key-for-protocol-partition partition)
partition-bits (or (oget obj partition-key) 0)]
(if (> partition-bits 0)
(let [lookup-table (get fast-path-protocols-lookup-table partition)
(let [lookup-table (get @fast-path-protocols-lookup-table partition)
_ (assert (map? lookup-table)
(str "fast-path-protocols-lookup-table does not contain lookup table for partition " partition))
* (fn [accum [bit protocol]]
Expand Down
25 changes: 25 additions & 0 deletions src/lib/devtools/optional.cljs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
(ns devtools.optional
(:require [cljs.pprint]
[cljs.stacktrace]))

; requires here do not play well with DCE in advanced mode
; see https://github.com/binaryage/cljs-devtools/issues/37
;
; this file exists to be required dynamically in dev mode only via
; (emit-if-compiler-in-dev-mode (goog/require "devtools.optional"))
;
; we rely on the fact that in :none optimizations mode clojurescript compiler produces all js files
; even if they are not reachable from :main namespace (if specified)
;
; please note that you should manually require namespaces which these requires depend on
; to achieve correct ordering of requires for static code, currently goog.string and clojure.string

; -- wrappers ---------------------------------------------------------------------------------------------------------------
;
; these are convenience wrappers, cljs.stacktrace/parse-stacktrace is a multi-method, so we couldn't call it directly from js

(defn pprint [& args]
(apply cljs.pprint/pprint args))

(defn parse-stacktrace [& args]
(apply cljs.stacktrace/parse-stacktrace args))
43 changes: 16 additions & 27 deletions src/lib/devtools/prefs.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -2,37 +2,26 @@
(:require-macros [devtools.prefs :refer [emit-external-config emit-env-config]])
(:require [devtools.defaults :as defaults]))

; we cannot use cljs.core/merge because that would confuse advanced mode compilation
; if you look at cljs.core/merge you will see that it relies on reduce which relies on protocol checks and
; this is probably too dymamic to be elided (my theory)
(defn simple-merge [base-map & maps]
(let [rmaps (reverse maps)
sentinel (js-obj)
sentinel? #(identical? % sentinel)
merged-keys (dedupe (sort (apply concat (map keys rmaps))))]
(loop [result base-map
todo-keys merged-keys]
(if (empty? todo-keys)
result
(let [key (first todo-keys)
val (first (remove sentinel? (map #(get % key sentinel) rmaps)))]
(recur (assoc result key val) (rest todo-keys)))))))

(def default-config defaults/prefs)
(def external-config (emit-external-config))
(def env-config (emit-env-config))
(def initial-config (simple-merge default-config external-config env-config))

(def ^:dynamic *prefs* initial-config)
; we use delay for DCE, see https://github.com/binaryage/cljs-devtools/issues/37
(def default-config (delay @defaults/config))
(def external-config (delay (emit-external-config)))
(def env-config (delay (emit-env-config)))
(def initial-config (delay (merge @default-config @external-config @env-config)))

(defn get-prefs []
*prefs*)
(def ^:dynamic *current-config* (delay @initial-config))

(defn pref [key]
(key *prefs*))
; -- public api -------------------------------------------------------------------------------------------------------------

(defn set-prefs! [new-prefs]
(set! *prefs* new-prefs))
(set! *current-config* new-prefs))

(defn get-prefs []
(when (delay? *current-config*)
(set-prefs! @*current-config*))
*current-config*)

(defn pref [key]
(key (get-prefs)))

(defn set-pref! [key val]
(set-prefs! (assoc (get-prefs) key val)))
Expand Down
9 changes: 9 additions & 0 deletions src/lib/devtools/util.clj
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,12 @@
(~f ~@args)
(catch :default e#
~exceptional-result)))

(defn compiler-in-dev-mode? []
(if cljs.env/*compiler*
(let [mode (get-in @cljs.env/*compiler* [:options :optimizations])]
(and (not= mode :advanced) (not= mode :simple)))))

(defmacro emit-if-compiler-in-dev-mode [body]
(if (compiler-in-dev-mode?)
body))
23 changes: 18 additions & 5 deletions src/lib/devtools/util.cljs
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
(ns devtools.util
(:require-macros [devtools.util :refer [oget ocall oset]])
(:require-macros [devtools.util :refer [oget ocall oset emit-if-compiler-in-dev-mode]])
(:require [goog.userAgent :as ua]
[clojure.data :as data]
[cljs.pprint :refer [pprint]]
[devtools.version :refer [get-current-version]]
[devtools.defaults :as defaults]
[devtools.context :as context]
[goog.string] ; for cljs.pprint, see devtools.optional
[clojure.string] ; for cljs.pprint, see devtools.optional
[devtools.prefs :as prefs]))

(emit-if-compiler-in-dev-mode (goog/require "devtools.optional"))

(def lib-info-style "color:black;font-weight:bold;")
(def reset-style "color:black")
(def advanced-build-explanation-url
Expand All @@ -17,6 +19,17 @@
(def ^:dynamic *console-open* false)
(def ^:dynamic *custom-formatters-warning-reported* false)

; -- general helpers --------------------------------------------------------------------------------------------------------

(defn pprint-str [& args]
; note that we have to do dynamic lookup here, cljs.pprint is present only in dev mode
; or when explicitly required by the user of cljs-devtools
(if (exists? js/devtools.optional.pprint)
(with-out-str
(binding [*print-level* 300]
(apply js/devtools.optional.pprint args)))
(apply pr-str args)))

; -- version helpers --------------------------------------------------------------------------------------------------------

(defn ^:dynamic make-version-info []
Expand Down Expand Up @@ -87,9 +100,9 @@

(defn print-config-overrides-if-requested! [msg]
(when (prefs/pref :print-config-overrides)
(let [diff (second (data/diff defaults/prefs (prefs/get-prefs)))]
(let [diff (second (data/diff @prefs/default-config (prefs/get-prefs)))]
(if-not (empty? diff)
(.info js/console msg (with-out-str (pprint diff)))))))
(.info js/console msg (pprint-str diff))))))

; -- custom formatters detection --------------------------------------------------------------------------------------------

Expand Down
13 changes: 7 additions & 6 deletions test/src/tests/devtools/tests/prefs.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@
[devtools.prefs :refer [merge-prefs! set-pref! set-prefs! update-pref! get-prefs pref]]))

(deftest test-default-prefs
(testing "default prefs should exist"
(is (> (count defaults/prefs) 0)))
(testing "some known default prefs values"
(is (= (:span defaults/prefs) "span"))
(is (= (:ol defaults/prefs) "ol"))
(is (= (:li defaults/prefs) "li"))))
(let [default-config @defaults/config]
(testing "default prefs should exist"
(is (> (count default-config) 0)))
(testing "some known default prefs values"
(is (= (:span default-config) "span"))
(is (= (:ol default-config) "ol"))
(is (= (:li default-config) "li")))))

(deftest test-changing-prefs
(testing "set prefs"
Expand Down
2 changes: 1 addition & 1 deletion test/src/tests/devtools/tests/utils/test.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
[clojure.string :as string]))

(defn reset-prefs-to-defaults! []
(set-prefs! defaults/prefs))
(set-prefs! @defaults/config))

; taken from https://github.com/purnam/purnam/blob/62bec5207621779a31c5adf3593530268aebb7fd/src/purnam/native/functions.cljs#L128-L145
; Copyright © 2014 Chris Zheng
Expand Down

0 comments on commit 30a3497

Please sign in to comment.