Fork me on GitHub
#cljs-dev
<
2016-11-09
>
darwin17:11:48

continued discussion from #clojurescript: have an interesting issue with :advanced mode build: https://gist.github.com/darwin/34b2eaf97f1823b1180eb14ad64af9fd initial-config is not used so it should be elided, 01 does not elide it, 02 does note the comment there

darwin17:11:15

interesting is case #03, which calls simple-merge twice but in parallel

darwin17:11:36

so nesting is not causing the issue, simply the fact that a function is called multiple times

darwin17:11:04

I think closure compiler decides to not inline it and that causes subsequent elision pass to fail to see that this whole thing can be elided

darwin17:11:33

just a theory of course 🙂

dnolen17:11:33

@darwin so your gist doesn’t really mean anything to me yet

dnolen17:11:40

if you use merge, why would it be elided?

dnolen17:11:07

I don’t understand what you are attempting to illustrate

darwin17:11:10

merge is NOT elided

dnolen17:11:31

but you used it

dnolen17:11:33

why should it be?

darwin17:11:49

because none of those defs is used anywhere in my scenario

darwin17:11:03

so everything should get elided, sorry, I didn’t explain it properly

darwin17:11:04

this is part of cljs-devtools, I want it to leave no trace in advanced builds, if not installed

dnolen17:11:04

ah - well cases 2) & 3) here don’t really seem that interesting to me

dnolen17:11:17

we can’t do much about the details of advanced compilation heuristics

darwin17:11:39

sure, that is why I didn’t want to report this as problem

darwin17:11:44

just interesting observation

dnolen17:11:59

as for 1) it would be nice to collect more information that the protocol test is the source of the problem

darwin17:11:17

I’m going to rewrite simple-merge to take multiple args, to see if that could fly

dnolen17:11:51

another issue here is that these assignments are in fact side effects

dnolen17:11:11

and require invoking other functions to compute their values

dnolen17:11:18

so this alone might damage the heuristic too much

dnolen17:11:29

we are very, very careful to avoid this in the standard library

darwin17:11:03

you mean the assignments is inside def?

dnolen17:11:15

I mean var x = foo();

dnolen17:11:38

it’s far better to assign an analyzable static value

dnolen17:11:16

we don’t have this pattern anymore of invoke + assign at the top level - it took very careful tiptoeing to support both dead code elimination and cross module code motion

darwin17:11:25

interesting, but that does not help me here, defaults/prefs is dynamic in my case, so I cannot merge those values at compile-time, I still need something like var x = merge(m1, m2, m3);

darwin17:11:30

I think I can work-around it, just by writing more flexible version of simple-merge and call it only once

dnolen17:11:56

@darwin another option is to just separate the configs from the fns (2 namespaces)

dnolen17:11:17

then DCE should work

dnolen17:11:27

because usage is explicit

darwin17:11:03

I’m afraid I don’t follow, are you suggesting that simple-merge should be moved to another namespace?

darwin17:11:27

almost works, but ::not-found keyword wasn’t elided

darwin17:11:34

everything else was

darwin17:11:56

interesting observation, namespaced keyword does not get elided but unqualified does

dnolen18:11:39

@darwin no I’m saying move those top level assignments

dnolen18:11:56

separate static analyzable stuff from stuff that’s dynamically computed at runtime

darwin18:11:00

btw. my theory is that when reduce or any other core function optionally calls some protocol methods, the closure compiler cannot be sure that there are no side-effects, so it cannot elide it, that is why I couldn’t use merge in the first place

darwin18:11:36

more puzzling is why 04 does not elide namespaced keyword, 06 is identical but with unqualified keyword and there elision works

darwin18:11:06

I worked around it by using (js-obj) sentinel which is more clean anyways

dnolen18:11:35

not interested in theories - interested in evidence 🙂

dnolen18:11:55

it should be trivial to write plain old javascript that replicates protocol pattern that demonstrates your theory holds

darwin19:11:54

ah, hit another problem, completely unrelated to the previous one is it possible to detect if my namespace was included via :preloads? the problem, normally people should install cljs-devtools by :preloads [devtools.preload] and using :main ‘root.ns, that is fine, but when they don’t and use no :preloads and no :main, then my devtools.preload gets executed at arbitrary point, because that ns is on classpath and without :main all namespaces are included in the build

darwin19:11:46

one option would be to move devtools.preload to some other folder not included by default, but then I would have to tell people to add this to their :source-paths when using :preloads

darwin19:11:59

which I don’t like

darwin19:11:53

to clarify: "but when they don’t and use no :preloads and no :main, then my devtools.preload gets executed at arbitrary point”, the proper behaviour I want is that devtools is not auto-installed at all, and must be installed by calling devtools.core/install! explicitly by the user (this was the original behaviour prior adding support for :preloads)

dnolen20:11:51

@darwin I do not understand what you are saying

dnolen20:11:58

we don’t include files which aren’t part of the build

darwin20:11:37

let me try to rephrase it: in problematic case, compiler options have no :main and no :preloads facts: 1. namespace devtools.preload is on classpath (it is always there because cljs-devtools was in dependencies) 2. currently looks like this https://github.com/binaryage/cljs-devtools/blob/master/src/lib/devtools/preload.cljs the problem: install! is called, because :main wasn’t specified

darwin20:11:36

if I were to specify :main, the devtools.preload namespace would not be reached, because nobody should require it, it sole purpose was to serve :preloads feature

dnolen20:11:15

@darwin you’re rephrasing isn’t helping

dnolen20:11:28

things are not loaded because they are on the classpath

dnolen20:11:02

they must be provided via inputs or via an explicit require somewhere

dnolen20:11:33

if something is getting pulled in just because it’s on the classpath something else is wrong

darwin20:11:02

I forgot to say that I’m now dealing with :advanced mode build

darwin20:11:54

but I thought this works the same with dev builds as well, missing :main means, treat all visible namespaces as “main” and include them

dnolen20:11:37

that’s not how it works

dnolen20:11:43

what I said above holds

dnolen20:11:57

we do not include random stuff on the classpath

darwin20:11:31

ok, maybe my terminology is wrong, what about :source-paths?

dnolen20:11:00

yes we automatically load anything in a directory provided by inputs

dnolen20:11:04

:source-paths isn’t a thing

dnolen20:11:15

just some lein-cljsbuild or other tooling stuff

dnolen20:11:58

in anycase, what you’re describing doesn’t seem like a problem for users

dnolen20:11:11

they won’t get your preloads from inputs / :source-paths they’ll get it from the classpath

dnolen20:11:26

:preloads only works via the classpath

darwin20:11:03

aha, now it makes more sense, thanks for the explanation

darwin20:11:40

so if I move preload.cljs out of this[1] folder, which is input, and manage to put it on classpath, then I will fix my issue: [1] https://github.com/binaryage/cljs-devtools/tree/master/src/lib/devtools

dnolen20:11:42

using :preloads with advanced is also not a thing

dnolen20:11:50

we emit a warning for that - we’re not going to support it ever

dnolen20:11:36

@darwin your conclusion about moving that file is correct

darwin20:11:21

thanks, will try it, today I learned about the distinction between classpath and input, good to know

darwin21:11:17

it is clear to me what I want to do, but unclear how to do that, how clojurescript compiler knows what is to be added into inputs when depending on a library? or this is responsibility of the build tool at hand? in case of leiningen, is cljsbuild plugin responsible for assembling inputs list by walking the dependency tree?

dnolen21:11:40

@darwin sorry for not being clear

dnolen21:11:47

first remove inputs from your mind

dnolen21:11:06

the only way anything can get into a build is if something requires it

dnolen21:11:14

if it is required it must exist on the classpath

dnolen21:11:35

now imagine inputs has one source file in it

dnolen21:11:53

we know everything in the build because we start with this one file and follow all of it’s requires

dnolen21:11:16

now imagine inputs has two source files in it

dnolen21:11:37

we use both of these files as entry points to figure out what’s in the build again by following requires (which are resolved on the classpath)

dnolen21:11:48

and so forth

dnolen21:11:41

inputs gives us entry points

dnolen21:11:48

but all resolution is via the classpath

darwin21:11:49

clear, thanks, so now, let’s add a library situation, clojurescript is not concerned about “maven” dependencies, right? it is the task of a build tool to prepare all inputs

dnolen21:11:13

nothing changes

dnolen21:11:32

everything I’ve said covers libraries

darwin21:11:32

what is unclear to me right now is how folders from my library’s jar file end up in inputs

dnolen21:11:43

they don’t end up in inputs

dnolen21:11:11

requires are resolved via the classpath

dnolen21:11:50

all Clojure tooling uses Maven to figure out the classpath when booting the JVM

dnolen21:11:38

a library JAR is just an entry on the classpath - that’s it

darwin21:11:24

well, now I understand that inputs is not list of folders, but list of files, right?

darwin21:11:22

when :main is specified, it is the file matching the namespace, if :main is not specified, what happens?

darwin21:11:26

wait, I have to think about it more, looks like those are two separate things

darwin21:11:59

maybe understanding this question would help me: what is the difference between leiningen’s root :source-paths and cljsbuild’s cljsbuild profile :source-paths

dnolen21:11:08

@darwin inputs can be files or folders, it doesn’t matter

dnolen21:11:30

I would leave :main out of this until you understand how this works

dnolen21:11:27

:source-paths is a separate discussion entirely

dnolen21:11:39

Leiningen root :source-paths is Maven source paths

darwin21:11:43

ok, without :main I believe I understand it, given inputs, walk all require deps and build list of all files for compilation, in topological sort

dnolen21:11:49

cljsbuild :source-paths is inputs

darwin21:11:03

we are getting closer, that is how I understood it before we started our discussion

darwin21:11:52

so, with this model in my head, my original situation with cljs-devtools library having all cljs files in one folder (including preload.cljs) should not be an issue, because library means just adding stuff on classpath, not into inputs

dnolen21:11:33

finally I can explain :main

dnolen21:11:40

inputs are your entry points

dnolen21:11:57

but if you provide :main there is only one - it must be a namespace on the classpath

dnolen21:11:15

it doesn’t matter what inputs is

darwin21:11:55

aha, got it

darwin21:11:52

if this is true, I must investigate why devtools.preload namespace gets added to the build, there must be something wrong with my project

darwin21:11:09

exactly what you wrote above

darwin21:11:55

found it[1], sorry for all the noise, I was 99% sure, I searched the whole project [1] https://github.com/binaryage/dirac/blob/master/src/implant/dirac/implant/deps.cljs

cschep21:11:32

; intentionally left empty ha ha

cschep21:11:37

good detective work!

darwin21:11:51

now it works, everything clicked