Fork me on GitHub
#cljs-dev
<
2018-09-07
>
mfikes01:09:44

Speedup of 1.13 for the last commit https://github.com/mfikes/cljs-perf

martinklepsch09:09:25

All this perf work is super awesome! 👏 Do you do any benchmarking of incremental builds as well?

mfikes21:09:20

@U050TNB9F No. These benchmarks (over the Coal Mine corpus) tend to run long enough to warm up the JVM, which I suspect accurately reflects what happens if you are using a long-lived Figwheel session. But building with cached artifacts is a whole other perf story I’m not currently measuring.

martinklepsch07:09:57

Yeah, was mostly thinking of code paths that are only run on incremental compilation. Thanks 🙏

mfikes01:09:24

That puts us at 2.26 relative to 1.10.339

parrot 16
dpsutton02:09:11

execution speed or compilation speed?

mfikes02:09:50

Compilation speed

Yehonathan Sharvit07:09:25

It seems that tools.reader 1.3.0 is not fully compatible with clojurescript 1.10.339 at least for reading namespace qualified keywords (in current namespace) (r/read-string "::foo") yields an exception. I can reproduce it in the cli, with this command:

clj -Sdeps '{:deps {org.clojure/clojurescript {:mvn/version "1.10.339"} org.clojure/tools.reader  {:mvn
/version "1.3.0"}}}' -m cljs.main --repl-env node -e "(ns foo.core) (require '[clojure.tools.reader :as r]) (r/read-string \"::foo\")"
I get this exception:
Exception in thread "main" clojure.lang.ExceptionInfo: /private/var/folders/l0/xhq1bf0x59lgpnnd7jglqb0r0000gn/T/out3192
375010680971978279779396035683/cljs/tools/reader.js:1604
throw e;
^

Error: Invalid token: ::foo

thheller07:09:55

@viebel auto-namespaced keywords require special bindings and are not valid edn

Yehonathan Sharvit07:09:15

@thheller What do you mean by special bindings?

Yehonathan Sharvit07:09:40

are you referrint to *alias-map*?

thheller07:09:16

I think so yes. can't remember the exact name.

thheller07:09:30

I think its *ns* in this case though

thheller07:09:46

*alias-map* is for readfing ::foo/bar

Yehonathan Sharvit07:09:35

This works:

clj -Sdeps '{:deps {org.clojure/clojurescript {:mvn/version "1.10.339"} org.clojure/tools.reader  {:mvn/version "1.3.0"}}}' -m cljs.main --repl-env node -e "(ns foo.core) (require '[clojure.tools.reader :as r]) (binding [*ns* 'aa] (r/read-string \"::foo\"))"

Yehonathan Sharvit07:09:19

Has something changed with clojurescript 1.10? In 1.9 it worked without any special bindings.

thheller07:09:21

not that I'm aware of no. what did 1.9 return if you don't actually bind *ns*?

Yehonathan Sharvit07:09:52

It returned cljs.user/foo

Yehonathan Sharvit07:09:45

I am not 100% sure. At least, in self-hosted cljs (e..g in Klipse) it worked

Yehonathan Sharvit11:09:22

Thanks @mfikes. The commit description is pretty clear: “fail on nil ns

dnolen13:09:03

@viebel probably how it worked before was accidental

dnolen13:09:19

since not that many people relied on that also unlikely to do anything about it

Yehonathan Sharvit13:09:46

No worries @dnolen I found a way to fix my code.

dnolen13:09:16

👍:skin-tone-4:

mfikes16:09:03

Found a perf boost for :parallel-build which ends up being 16% faster on smaller codebases (like the compiler's unit tests, for example) Doing some more testing to ensure single-threaded perf doesn't regress. https://dev.clojure.org/jira/browse/CLJS-2896 Fairly simple change: https://github.com/mfikes/clojurescript/commit/fc66a5a558b2749c93e1d29605bb69d58f0ad36f

dnolen17:09:09

@mfikes hrm interesting, so not stalling on io

mfikes17:09:22

Yeah, and perhaps another reason is simply a little more parallelism, especially if you have more cores than files

dnolen17:09:03

I’d be surprised if that’s not faster for even larger code bases?

mfikes17:09:22

Well, for something like Coal Mine, there are a lot of files, so the cores are kept busy, and where you really see the difference is in that rampdown that occurs at the end, when there are straggler files still being built

mfikes17:09:50

I’m nearly done testing it, and will make the patch available

mfikes17:09:26

But, yeah, I don’t fully appreciate all of the consequences yet 🙂

mfikes17:09:28

FWIW, I tried adding parallel reading, but that made no difference as reading is so fast

mfikes17:09:38

Cool, patch is now in https://dev.clojure.org/jira/browse/CLJS-2896 if anyone cares to try it on their codebase

dnolen17:09:06

@mfikes nice I now consistently see basic compile times under 9 seconds on my desktop machine for the runtime tests

dnolen17:09:30

approved & re-assigned

kucerm218:09:11

@mfikes hi, my transaction ID of CA is CBJCHBCAABAAr-OiYOiJgrZUTHv2MzSVy9txqxESM6hY

mfikes19:09:45

@jmb Thanks for the patch in CLJS-2849. This appears to be your first contribution. Have you submitted the CA?

mfikes19:09:41

@kucerm2 I left a comment for you in https://dev.clojure.org/jira/browse/CLJS-2868 (For some reason I can’t assign the ticket to you.)

richiardiandrea22:09:27

does ClojureScript read deps.cljs from :local/root dependencies?