Skip to content

Commit

Permalink
add option to minimize require calls for imported JS deps
Browse files Browse the repository at this point in the history 
using the full path generates too much code for libs that have
hundreds or more required files. using numeric ids like webpack
hurts caching a bit but can reduce build size by quite a bit.
  • Loading branch information
@thheller
thheller committed Feb 6, 2019
1 parent 34f089a commit c2abeaf
Show file tree
Hide file tree
Showing 8 changed files with 169 additions and 32 deletions.
2 changes: 1 addition & 1 deletion shadow-cljs.edn
View file
Expand Up @@ -193,7 +193,7 @@
:devtools
{:http-root "out/demo-browser/public"
:http-port 8600
:loader-mode :eval
;; :loader-mode :eval
:repl-init-ns demo.browser
:before-load-async demo.browser/stop-from-config
:after-load demo.browser/start-from-config}}
Expand Down
2 changes: 2 additions & 0 deletions src/main/shadow/build.clj
View file
Expand Up @@ -298,6 +298,8 @@
:elide-asserts true
:load-tests false
:pretty-print false})
(build-api/with-js-options
{:minimize-require true})
(update-in [:compiler-options :closure-defines] merge {'goog.DEBUG false}))

closure-defines
Expand Down
68 changes: 44 additions & 24 deletions src/main/shadow/build/closure.clj
View file
Expand Up @@ -1620,7 +1620,21 @@
(let [rc-id
(data/get-source-id-by-provide state ns)
{:keys [resource-name] :as rc}
(data/get-source-by-id state rc-id)]
(data/get-source-by-id state rc-id)

require-map
(if-not (get-in state [:js-options :minimize-require])
require-map
(reduce-kv
(fn [m require target]
(let [src-id (get sym->id target)
src (get-in state [:sources src-id])]
(if-not (and src (:require-id src))
m
(assoc m require (:require-id src)))))
require-map
require-map))]

(assoc m resource-name require-map)))
{}
str->sym))
Expand All @@ -1629,10 +1643,14 @@
"takes a list of :npm sources and rewrites in a browser compatible way, no full conversion"
[{:keys [project-dir js-options npm mode build-sources] :as state} sources]
(let [source-files
(->> (for [{:keys [resource-id resource-name ns file deps] :as src} sources]
(->> (for [{:keys [resource-id resource-name ns require-id file deps] :as src} sources]
(let [source (data/get-source-code state src)]
(closure-source-file resource-name
(str "shadow$provide[\"" ns "\"] = function(global,process,require,module,exports,shadow$shims) {\n"
(str "shadow$provide["
(if (and require-id (:minimize-require js-options))
(pr-str require-id)
(str "\"" ns "\""))
"] = function(global,process,require,module,exports,shadow$shims) {\n"
(if (str/ends-with? resource-name ".json")
(str "module.exports=(" source ");")
source)
Expand Down Expand Up @@ -1680,6 +1698,11 @@
property-collector
(PropertyCollector. cc)


replace-require-pass
(let [m (require-replacement-map state)]
(ReplaceRequirePass. cc m))

closure-opts
(doto (make-options)
(set-options co-opts state)
Expand All @@ -1697,9 +1720,7 @@
(.addCustomPass CustomPassExecutionTime/BEFORE_CHECKS
(NodeStuffInlinePass. cc))

(.addCustomPass CustomPassExecutionTime/BEFORE_CHECKS
(let [m (require-replacement-map state)]
(ReplaceRequirePass. cc m)))
(.addCustomPass CustomPassExecutionTime/BEFORE_CHECKS replace-require-pass)

(.setWarningLevel DiagnosticGroups/NON_STANDARD_JSDOC CheckLevel/OFF)
(.setWarningLevel DiagnosticGroups/MISPLACED_TYPE_ANNOTATION CheckLevel/OFF)
Expand Down Expand Up @@ -1736,7 +1757,10 @@
_ (throw-errors! state cc result)

source-map
(.getSourceMap cc)]
(.getSourceMap cc)

alive-replacements
(.getAliveReplacements replace-require-pass)]

(-> state
(log-warnings cc result)
Expand All @@ -1759,23 +1783,6 @@
(catch Exception e
(throw (ex-info (format "failed to generate JS for \"%s\"" name) {:name name} e))))

;; the :simple optimization may remove conditional requires
;; the JS inspector is not smart enough to detect this without optimizing itself
;; so instead just check if the compiled JS still contains the module name
;; the name is unique enough so it shouldn't run into something the user actually typed
;; module$node_modules$react$cjs$react_production_min
;; it is not done as a compiler pass since I cannot figure out how to do it
;; the require("thing") is renamed to b("thing") so I can't check NodeUtil.isCallTo("require")
;; no idea if I can get the original name after renaming, its not always b so can't use that
;; anyways this should be good enough and fixes the react conditional require issue
removed-requires
(->> (get-in state [:str->sym ns])
(vals)
;; test at least for ("module$thing")
;; so it doesn't conflict with module$thing$b
(remove #(str/includes? js (str "(\"" % "\")")))
(into #{}))

sm-json
(when generate-source-map?
(let [sw (StringWriter.)]
Expand All @@ -1787,6 +1794,19 @@
deps
(data/deps->syms state rc)

;; conditional requires may be removed by :simple optimizations
;; the ReplaceRequirePass remembers all the require calls it modified
;; and after optimizations we can extract which ones are still alive
removed-requires
(->> (get-in state [:str->sym ns])
(vals)
(remove (fn [require-ns]
(or (contains? alive-replacements require-ns)
(let [require-id (get-in state [:sym->require-id require-ns])]
(and require-id (contains? alive-replacements require-id))))))
(into #{}))


actual-requires
(into #{} (remove removed-requires) deps)

Expand Down
57 changes: 55 additions & 2 deletions src/main/shadow/build/closure/ReplaceRequirePass.java
View file
Expand Up @@ -4,18 +4,34 @@
import com.google.javascript.rhino.IR;
import com.google.javascript.rhino.Node;

import java.util.Map;
import java.util.*;

public class ReplaceRequirePass extends NodeTraversal.AbstractPostOrderCallback implements CompilerPass {

private final AbstractCompiler compiler;
private final Map<String, Map<String, Object>> replacements;

public final List<ChangedRequire> changedRequires = new ArrayList<>();

public ReplaceRequirePass(AbstractCompiler compiler, Map<String, Map<String, Object>> replacements) {
this.compiler = compiler;
this.replacements = replacements;
}

public static class ChangedRequire {
public final Node requireNode;
public final String sourceName;
public final String require;
public final Object replacement;

public ChangedRequire(Node requireNode, String sourceName, String require, Object replacement) {
this.requireNode = requireNode;
this.sourceName = sourceName;
this.require = require;
this.replacement = replacement;
}
}

@Override
public void visit(NodeTraversal t, Node node, Node parent) {
if (NodeUtil.isCallTo(node, "require")) {
Expand All @@ -39,7 +55,18 @@ public void visit(NodeTraversal t, Node node, Node parent) {
// might be a clj-sym or String
Object replacement = requires.get(require);
if (replacement != null) {
requireString.replaceWith(IR.string(replacement.toString()));
Node replacementNode = null;

if (replacement instanceof Long) {
replacementNode = IR.number((Long) replacement);
} else {
replacementNode = IR.string(replacement.toString());
}

requireString.replaceWith(replacementNode);

changedRequires.add(new ChangedRequire(node, sfn, require, replacement));

t.reportCodeChange();
}
}
Expand All @@ -49,6 +76,32 @@ public void visit(NodeTraversal t, Node node, Node parent) {
}
}

public static boolean isAlive(Node node) {
Node test = node;
while (true) {
if (test == null) {
return false;
} else if (test.isRoot()) {
break;
}
test = test.getParent();
}
return true;
}

// only call this after optimizations are done, otherwise everything will be alive
public Set<Object> getAliveReplacements() {
Set<Object> alive = new HashSet<>();

for (ChangedRequire req : changedRequires) {
if (isAlive(req.requireNode)) {
alive.add(req.replacement);
}
}

return alive;
}

@Override
public void process(Node externs, Node root) {
NodeTraversal.traverse(compiler, root, this);
Expand Down
58 changes: 56 additions & 2 deletions src/main/shadow/build/compiler.clj
View file
Expand Up @@ -1025,10 +1025,17 @@
to ensure recompilation when their names change"
[state resource-id]
(let [rc (data/get-source-by-id state resource-id)
deps-syms (data/deps->syms state rc)]
deps-syms (data/deps->syms state rc)

require-ids
(->> deps-syms
(map #(get-in state [:sym->require-id %]))
(remove nil?)
(into #{}))]

(update-in state [:sources resource-id :cache-key]
(fn [key]
(->> (into key deps-syms)
(->> (into key (concat deps-syms require-ids))
(distinct)
(into []))))))

Expand All @@ -1042,6 +1049,48 @@
:file file
:js-errors merged})))))

(defn assign-require-ids [state source-ids]
(loop [state state
[src-id & more] source-ids
idx 0]
(if-not src-id
state
(let [{:keys [ns type] :as src} (get-in state [:sources src-id])]
(if (not= :shadow-js type)
(recur state more idx)

;; assign an alias to be used when setting up the provide and on each require
;; shadow$provide[some-alias] = function(...) { require(some-alias); }
;; typically defaults to the full namespace which has a pretty big impact on overall
;; size on libraries that contains a lot of small files that all require each other in some way
;;
;; a simple numeric id is the best for overall size (after gzip) but doesn't cache well
;; since every added JS dep may shift the ids and thus require recompiles
;;
;; using the file ns in hash form is overall shorter than most paths but downright hostile
;; when it comes to gzip compression and thus not viable
;;
;; even a shortened hashes still basically nullify gains in gzip
;;
;; full hash [JS: 825.62 KB] [GZIP: 205.61 KB]
;; full path [JS: 891.56 KB] [GZIP: 186.04 KB]
;; short hash 8 [JS: 745.50 KB] [GZIP: 184.36 KB] (first 8 chars)
;; short hash 6 [JS: 738.83 KB] [GZIP: 182.84 KB] (first 6 chars)
;; numeric id [JS: 719.89 KB] [GZIP: 177.36 KB]
;;
;; don't know how short the hash could be to reduce conflicts since each conflict
;; would mean we need to recompile which then makes it worse than numeric ids
;;
;; wonder if there is something that is gzip-friendly but also cache-friendly

(let [alias idx #_ (subs (util/md5hex (str ns)) 0 6)]
(-> state
(update-in [:sources src-id] assoc :require-id alias)
(assoc-in [:require-id->sym alias] ns)
(assoc-in [:sym->require-id ns] alias)
(recur more (inc idx))
)))))))

(defn compile-all
"compile a list of sources by id,
requires that the ids are in dependency order
Expand All @@ -1057,6 +1106,11 @@
(cljs-hacks/install-hacks!)

(let [state
(if-not (get-in state [:js-options :minimize-require])
state
(assign-require-ids state source-ids))

state
(reduce ensure-cache-invalidation-on-resolve-changes state source-ids)

sources
Expand Down
4 changes: 4 additions & 0 deletions src/main/shadow/build/data.clj
View file
Expand Up @@ -29,6 +29,10 @@
;; since closure only works with names not ids
:name->id {}

;; numeric require mapped to its namespace and back
:require-id->sym {}
:sym->require-id {}

;; map of {resource-id #{resource-id ...}}
;; keeps track of immediate deps for each given source
;; used for cache invalidation and collecting all deps
Expand Down
8 changes: 6 additions & 2 deletions src/main/shadow/build/npm.clj
View file
Expand Up @@ -690,7 +690,7 @@
:require require
:override override})))))))

(defn shadow-js-require [{:keys [ns resource-config] :as rc}]
(defn shadow-js-require [{:keys [ns require-id resource-config] :as rc}]
(let [{:keys [export-global export-globals]}
resource-config

Expand All @@ -712,7 +712,11 @@
(seq globals)
(assoc :globals globals)))]

(str "shadow.js.require(\"" ns "\", " (json/write-str opts) ");")))
(str "shadow.js.require("
(if require-id
(pr-str require-id)
(str "\"" ns "\""))
", " (json/write-str opts) ");")))

;; FIXME: allow configuration of :extensions :entry-keys
;; maybe some closure opts
Expand Down
2 changes: 1 addition & 1 deletion src/main/shadow/js.js
View file
Expand Up @@ -90,7 +90,7 @@ shadow.js.jsRequire = function(name, opts) {

try {
if (goog.DEBUG) {
if (name.indexOf("/") != -1) {
if (name instanceof String && name.indexOf("/") != -1) {
console.warn(
"Tried to dynamically require '" +
name +
Expand Down

0 comments on commit c2abeaf

Please sign in to comment.