Fork me on GitHub
#cljs-dev
<
2018-06-30
>
kommen07:06:41

@mfikes do you have any infrastructure for benchmarking cljs patches with all the different js engines or do you just run them locally?

mfikes11:06:02

@kommen Yes, use script/benchmark First set up engines per https://clojurescript.org/community/running-tests I clear all of the existing tests out of the benchmark/cljs/benchmark_runner.cljs and add my own. Then, using a calculator, I calculate speedup numbers per engine.

mfikes11:06:33

(Speedup numbers are the updated time divided by the original time.)

ambrosebs19:06:31

@mfikes I did some further investigation of that strange behaviour where the bootstrapped cljs.compiler couldn't dispatch to JSValue. It seems that simulating that dispatch works perfectly fine in cljs.analyzer, but consistently fails in cljs.compiler. Here's a minimal commit (doesn't include any other experimentation) that shows the discrepancy with ./scripts/test-self-parity https://github.com/clojure/clojurescript/compare/master...frenchy64:bootstrap-jsvalue

ambrosebs19:06:47

the output is consistently:

ambrosebs19:06:53

"ANALYZER JSValue dispatch worked!"
"COMPILER JSValue dispatch failed"

ambrosebs21:06:58

It looks like JSValue is "compiled twice" in self-hosted, with (hash JSValue) varying over #{16 17}. In the code above, cljs.analyzer just happens to have methods for both versions (somehow), and cljs.compiler only has a method for the hash 17. New instances seem to always have (hash (type x)) = 16. I'll try and poke around some more.

mfikes21:06:52

@ambrosebs Yes. I see the same. And, at its root, the method table that gets built in cljs.compiler has the one with hash 17. The double loading of cljs.tagged-literals is particular bad given the consequence—it is probably a disconnect between the code related to CLOSURE_IMPORT_SCRIPT in self-parity.test trying to track things (which really reflects an approach used by JVM-based REPLs), and cljs.js/*loaded* being the main self-hosted mechanism for the same.

mfikes21:06:14

@ambrosebs If you modify self-parity.test/skip-load? by adding 'cljs.tagged-literals to the “non-macros” set in that impl, it would fix things. (Arguably, things are a mess in there with respect to loading, but that one spot is where we’ve “hacked” things to keep stuff from being loaded that is already loaded.)

mfikes22:06:26

Not arguing that this is a clean way to solve it… hacks unfortunately abound in this bit of code, and bite back every now and then 😞

ambrosebs22:06:26

🙂 thanks! I'll try it out

ambrosebs23:06:58

Yep that worked. In other news I also got tools.analyzer-style :children vectors working (YES!)

🎉 4