Fork me on GitHub
#cljs-dev
<
2017-07-02
>
mfikes00:07:55

The aget / aset warning patch found 15 abuses in the std lib and a handful more in the test suite. (Supplied fixes for those in a revised patch in CLJS-2148)

dnolen00:07:16

@mfikes might be nice to have those in separate patch

mfikes00:07:29

OK. Will split them into two.

dnolen00:07:41

this new warning is guaranteed to create chaos 馃檪

dnolen00:07:52

probably one of the few warnings we want to put behind a false flag

dnolen00:07:57

and people can enable it for code style

mfikes00:07:14

Yes, and it would be nice to separate the warning itself from all of the (perhaps slightly dangerous) fixes for the warning.

mfikes00:07:40

Ahh, yeah, we can have them default to false.

mfikes00:07:10

As David suggested, there is now a CLJS-2148-v2.patch attached to https://dev.clojure.org/jira/browse/CLJS-2148 that you can try on your codebases to see how many abuses of aget and aset it can find. The warnings are turned off by default, so you'd need to enable them.

mfikes12:07:29

If code uses aget / aset for object access, then it makes it difficult to do anything about https://dev.clojure.org/jira/browse/CLJS-2149

mfikes12:07:44

Not impossible; just difficult to do efficiently while preserving abuse

mfikes12:07:50

Perhaps the code emitted could depend on what type the analyzer infers. That would be a mess, but would shove some of the inefficiency out of runtime to compile time.

dnolen16:07:57

I鈥檝e tested the module-loader-preflight branch quite a bit - all tests (core, lein, self-host, self-parity), all REPLs (browser, nashorn, rhino, node), JS module guides, on Om (w/ Figwheel), on a trivial cljsjs.react project and I tested this new guide of course.

dnolen16:07:38

but more testing would be good as it required touching quite a bit of code so that we could compute the information required by ModuleManager in straightforward way.

dnolen16:07:10

while thinking through the :none where we need to load stuff dynamically - I went ahead and re-considered what @thheller said about assigning compiler inputs to modules - and I copied what Google Closure JSModuleGraph does more or less.

dnolen16:07:13

pretty huge change so I鈥檓 less concerned about code review and more just testing 馃檪

dnolen16:07:37

also can鈥檛 believe I just realized that java -cp cljs.jar clojure.main -m cljs.repl.browser works for starting a local webserver 馃檪

mfikes16:07:06

AFK, but wondering if java -jar cljs.jar -m cljs.repl.browser also works :)

dnolen16:07:49

right, we should make this a hair nicer so you can get just the simple webserver and not the REPL

dnolen16:07:36

I鈥檝e been instinctively using python -m SimpleHTTPServer 8000 but this isn鈥檛 so nice for tutorials

mfikes17:07:34

The above looks like really cool stuff. I'm wondering if might even be useful for gigantic React Native apps (where all the code is local, but you want to bring up the initial screen with minimal latency. I doubt it would matter for that use case in the end, given how quickly JavaScript can be loaded. Also wondering about self-host low-latency loading (understood if none of this works in self-hosted).

dnolen17:07:11

hrm hard to say - code splits are driven by network latency - so your parse times would probably be have to be worse than that to be impactful?

anmonteiro18:07:53

Anyone seen this before?

dnolen18:07:47

@anmonteiro no but the error is quite strange

dnolen18:07:55

it looks like it tried to find a macro in a .cljs file no?

anmonteiro18:07:19

Looks like it. I wonder if we still have a bug in cljs.js-deps

dnolen18:07:10

@anmonteiro is this with master?

anmonteiro18:07:23

I'm also going to disable :parallel-build from now on since it could be the case that there's a bug in there

anmonteiro18:07:30

This is with 1.9.671

dnolen18:07:53

hrm, I don鈥檛 see how that could cause macro lookup in the wrong file type (but maybe)

anmonteiro18:07:07

Trying to get the new Lumo release through Homebrew

anmonteiro18:07:48

It only ever seems to happen on OSX Yosemite though

anmonteiro18:07:07

I don't know what kind of conditions are in place for this to be happening

dnolen18:07:26

well hopefully someone else can repro

dnolen18:07:54

I mean seems common enough situation that someone else might run into it

bronsa18:07:39

I think we saw something similar in one of our builds the other day

bronsa18:07:51

don't have access to the build logs ATM but will check tomorrow

anmonteiro18:07:07

just verified it happens on Windows too (and macOS Sierra). opened https://dev.clojure.org/jira/browse/CLJS-2153

anmonteiro18:07:15

here鈥檚 the Windows build where the problem occurs: https://ci.appveyor.com/project/anmonteiro/lumo/build/815

anmonteiro18:07:33

java.lang.Thread.run              Thread.java:  745
 java.util.concurrent.ThreadPoolExecutor$Worker.run  ThreadPoolExecutor.java:  617
  java.util.concurrent.ThreadPoolExecutor.runWorker  ThreadPoolExecutor.java: 1142
                                                ...                               
                          clojure.core/bound-fn*/fn                 core.clj: 2000
                                 clojure.core/apply                 core.clj:  661
                                                ...                               
                        clojure.core/with-bindings*                 core.clj: 1970 (repeats 2 times)
                                 clojure.core/apply                 core.clj:  657
                                                ...                               
           cljs.closure/parallel-compile-sources/fn              closure.clj:  889
                          cljs.closure/compile-task              closure.clj:  858
                       cljs.closure/compile-task/fn              closure.clj:  862
                         cljs.closure/eval6343/fn/G              closure.clj:  450
                           cljs.closure/eval6413/fn              closure.clj:  576
                      cljs.closure/compile-from-jar              closure.clj:  548
                         cljs.closure/eval6343/fn/G              closure.clj:  450
                           cljs.closure/eval6407/fn              closure.clj:  566
                          cljs.closure/compile-file              closure.clj:  497
                         cljs.compiler/compile-file            compiler.cljc: 1436
                      cljs.compiler/compile-file/fn            compiler.cljc: 1459
                        cljs.compiler/compile-file*            compiler.cljc: 1370
                       cljs.compiler/with-core-cljs            compiler.cljc: 1206
                     cljs.compiler/compile-file*/fn            compiler.cljc: 1381
                          cljs.compiler/emit-source            compiler.cljc: 1307
                              cljs.analyzer/analyze            analyzer.cljc: 3352
                             cljs.analyzer/analyze*            analyzer.cljc: 3328
                                clojure.core/reduce                 core.clj: 6703
                                                ...                               
                          cljs.analyzer/analyze*/fn            analyzer.cljc: 3328
                      cljs.analyzer/ns-side-effects            analyzer.cljc: 3265
   cljs.analyzer/check-use-macros-inferring-missing            analyzer.cljc: 2055
                                                ...                               
                             clojure.core/update-in                 core.clj: 6054
                             clojure.core/update-in                 core.clj: 6068
                          clojure.core/update-in/up                 core.clj: 6067
                                 clojure.core/apply                 core.clj:  659
                                                ...                               
cljs.analyzer/check-use-macros-inferring-missing/fn            analyzer.cljc: 2057
                     cljs.analyzer/check-use-macros            analyzer.cljc: 2043
                                cljs.analyzer/error            analyzer.cljc:  655
                                cljs.analyzer/error            analyzer.cljc:  657
                               clojure.core/ex-info                 core.clj: 4725
clojure.lang.ExceptionInfo: Invalid :refer, macro cljs.spec.gen.alpha/lazy-prims does not exist in file C:\Users\appveyor\.boot\cache\tmp\projects\lumo\18g\-gi273p\main.out\cljs\spec\gen\alpha.cljs

anmonteiro18:07:39

so ^ here鈥檚 the stacktrace

anmonteiro18:07:57

seems related to the analyzer and not js-deps

thheller18:07:35

@dnolen I don鈥檛 see a :module-loader config option in your example? I think the loader should be optional since it adds quite a few dependencies itself

anmonteiro18:07:45

I wonder if there could be some sort of concurrency bug in acquiring these locks: https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/analyzer.cljc#L3244-L3262

anmonteiro18:07:59

sounds unlikely, but I don鈥檛 have any other ideas ATM

rauh19:07:54

Further working on apply speedup. It's now beating CLJ in a few cases 馃檪

dnolen19:07:01

@thheller it is optional your ns has to require it. No need for flag.

thheller19:07:21

ah, good point

mfikes23:07:25

@anmonteiro if the concurrency thing is relatively reproducible, perhaps a git bisect, or a coarse-grained "first compiler version" would let us pinpoint. I was looking at this recent commit, wondering if the ::no-resolve could derail things somehow in concurrent code https://github.com/clojure/clojurescript/commit/01a1427e45c9726244940baeeb37b7511acbd8b1

mfikes23:07:17

(Maybe I can try to repro when I again have access to my 12-core box.)

anmonteiro23:07:17

that doesn鈥檛 look very offensive to me

anmonteiro23:07:52

the problem with this issue is that it鈥檚 non-deterministic (hence me thinking about parallel build)

anmonteiro23:07:59

I can鈥檛 repro it locally for example

anmonteiro23:07:05

so far it has only shown up in CI

mfikes23:07:17

That reminds me, I've turned off :parallel-build for Planck owing to a fairly reproducible Transit-related bug I need to track down. Maybe I can spend some time with Lumo with :parallel-build as well and try to shake out any concurrency bugs.

anmonteiro23:07:52

by all means 馃檪