This page is not created by, affiliated with, or supported by Slack Technologies, Inc.
2017-06-17
Channels
- # beginners (10)
- # boot (9)
- # cider (1)
- # cljs-dev (191)
- # clojure (77)
- # clojure-russia (4)
- # clojure-serbia (10)
- # clojure-spec (19)
- # clojure-uk (4)
- # clojurescript (16)
- # core-async (9)
- # cursive (1)
- # data-science (1)
- # datomic (3)
- # emacs (42)
- # graphql (2)
- # hoplon (38)
- # jobs (1)
- # jobs-discuss (18)
- # luminus (3)
- # lumo (20)
- # off-topic (9)
- # om (51)
- # parinfer (3)
- # pedestal (8)
- # re-frame (4)
- # reagent (7)
- # rum (9)
- # sql (9)
- # untangled (81)
(def ^:const a-const {:hello ["something" {:really #{:nested 1}}]})
(js/console.log "identical?" (identical? a-const a-const));; => false
maybe I misunderstand the goal of :const
but I would expect a constant to always be identical with itself?
27 commits to master on Friday.
This seems to break the :const
stuff, I get a "Failed compiling" without a more specific error message:
(def ^:const content-type-header
"A JS object with Content-Type key and the transit header."
#js{"Content-Type" "application/transit+json; charset=utf-8"})
@thheller it's just for substitutions and I don't think the identical? property is important
Sorry, I made a mistake above, It should be: (def ^:const some-var (some-ns/some-macro))
(def ^:const foo (do (prn :foo) 1))
this also doesn’t look like a const since the prn
is triggered every time it is inlined
I think clojure just precomputes :const
during compilation stage, kinda like a macro except it's not called. Eg this code: https://gist.github.com/rauhs/fbf9e184d41447daf7d90101a463766a
@rauh we could try to evaluate the constant initializer but I don't really care about that use case.
Yeah it's not important, GCC will do the precomputing for us. And it's been inlining all the @const
.
Just think, that the inlining should be maybe limited, to strings/numbers/regexes (etc)?
not sure what redefs have to do with it but if you say it is working as intended I take you word for it
This change will need a big warning message upon release or we'll have the channels flooded with errors. I needed to remove quite a bit of :const
for my project to compile
I don't really think Clojure behavior here is replicatable because of runtime compile time divide
Related I wonder if @nosideeffect annotation in top level literals would fix our DCE issues?
No, there is no way to hint @nosideeffect
, the official response is that "GCC should be able to detect this", so the annotation isn't allowed
Yeah it's terrible with cljs code at detecting this. Even a (concat [] ["a"])
stays in there.
Just catching up on the above after having filed https://dev.clojure.org/jira/browse/CLJS-2097. In this case I had ported some working Clojure code which was using ^:const
(https://github.com/AvisoNovate/pretty/blob/master/src/io/aviso/ansi.clj#L8-L20) to ClojureScript. Perhaps we are considering this might be invalid code in ClojureScript?
@mfikes yeah I think after @rauh’s report only attempting to inline actual static values is probably the way to go
Ahh, so the scope of the optimization might be reduced from EDN to literals like 1
and "a"
?
Or, are we saying that the code is invalid? For example (def ^:const (str a))
should not have the ^:const
on it.
we should definitely adjust our expectations about what Closure can do with top level ClojureScript data structures now - like I said I could not recreate the old problems here anyway with trivial examples
I agree, it used to be much worse. Now it seems the DCE does remove top level data structures.
Even if it turns out to not be feasible to detect clean EDN, something along these lines might still be useful
- (when const?
+ (when (and const? ('#{boolean number string} (:tag init-expr)))
more specifically it doesn’t get removed because of the .call
usually ends up as something like Nc||Mc.call(null,1);
Yeah I've initially considered adding that to that deref ticket from yesterday. But kept the scope small.
it doesn’t get removed because it goes through the cljs.core.atom
dispatch fn and not the cljs.core.atom.cljs$core$IFn$_invoke$arity$1
@thheller Btw, the patch in 1992 has the filename 1994, and it doesn't apply to master right now
@rauh @mfikes try master when you get a chance, hopefully it means you don’t have to change anything
On the side, I'm seeing if I can break case
in any way with this new feature. It is nice: The test values put into the switch are inlined 🙂
cljs.user=> (source atom)
(declare create-inode-seq create-array-node-seq reset! create-node atom deref)
@thheller I evidently failed to file an upstream bug when I noticed that 😞 https://github.com/mfikes/planck/issues/188
This:
(def ^:const path-mask 63)
(defn path-get
"Gets inlined by GCC"
[path level]
(bit-and path-mask
(unsigned-bit-shift-right path level)))
PRoduces:
eav.btset.path_get = (function eav$btset$path_get(path,level){
return ((63);
& (path >>> level));
});
@mfikes it would be fixed by https://dev.clojure.org/jira/browse/CLJS-1992 don’t think it needs a new issue
(ns test.foo)
(defn atom-sig
([x] x)
([x & {:keys [meta validator]}] [x meta validator]))
gets removed completelyscript/test-self-parity
is failing right now, will see which commit regressed it: https://gist.github.com/mfikes/f6e7caa45a026608621a459cb703ff75
Planck is also failing to compile in an odd way that I haven't sorted with the last commit
@dnolen Things are clean for me now. The case
I have this blog post runs an order of magnitude faster now http://blog.fikesfarm.com/posts/2015-06-15-clojurescript-case-constants.html (putting together a gist showing that)!
case
used to compile down to use the JavaScript variables for ^:const
expressions as tests. Now they are inlined with the literal values
I didn't make any changes to case
, historically, that I recall. Perhaps someone made it use JavaScript switch
constructs under some conditions... but yeah, the main speedup here is due to the inlining of constants.
@mfikes oh hrm, I thought that’s what you said yesterday that you did something with ^:const
it’s true in theory that Google Closure should do @const
replacement, but it’s nice to just get this feature without going :advanced
You could use the fact that it won't generate proper calls in clojure/cljs/compiler_tests.clj
I just did some of those tests in https://dev.clojure.org/jira/browse/CLJS-2046
(deftest test-cljs-1992 ;; declare after def should have no effect
(let [test-cenv (e/default-compiler-env)]
(e/with-compiler-env test-cenv
(a/analyze-form-seq
'[(ns test.cljs-1992)
(defn test-fn [a b c] :foo)
(declare test-fn)]
))
(let [def (get-in @test-cenv [::a/namespaces 'test.cljs-1992 :defs 'test-fn])]
(is (:fn-var def)))))
@dnolen updated with test https://dev.clojure.org/jira/browse/CLJS-1992
@rauh nice the patch seems to collapse the difference between (get x :foo)
and (:foo x)
@rauh while I’m testing this, I think the name of the config is misleading :fn-invoke-call
Yeah IIRC, my inital suggestion was something like assume-fast-invoke
or something like it.
@mfikes I’m somewhat curious if 2046 affects compile time significantly, if you can run your compile time thing that would be cool 🙂
I looked through the code trying to find anything that could be turned into a ^:const
and this was the only thing, but to truly see if it does anything, it might only improve self-hosted compilation speeds (which I haven't checked): https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/compiler.cljc#L367
I didn't see significant increases in compile time with 2046, I was wondering that too. It's pretty rare to do a higher order invoke like that.
The only thing is that people should prefer to do (get @x y)
instead of (@x y)
because of the potential IIFE, but they're also very fast so end result should still be faster than going thru the dispatch
@dnolen CLJS-2046 doesn't seem to affect the time needed to compile Planck. (Around 15 seconds both with and without the change.)
@dnolen With the change from Thomas, this becomes obsolete: https://dev.clojure.org/jira/browse/CLJS-2096
Got some CI sorted out with an initial patch in https://dev.clojure.org/jira/browse/CLJS-2098. Here is what it looks like running on a fork, if you're curious: https://travis-ci.org/mfikes/clojurescript Credit to Rohit. And António has ideas surrounding Docker that could make it better.