This page is not created by, affiliated with, or supported by Slack Technologies, Inc.
2019-07-11
Channels
- # announcements (1)
- # aws (3)
- # beginners (48)
- # calva (2)
- # cider (47)
- # clj-kondo (1)
- # cljs-dev (23)
- # cljsrn (10)
- # clojure (81)
- # clojure-chicago (4)
- # clojure-europe (3)
- # clojure-greece (4)
- # clojure-italy (8)
- # clojure-losangeles (1)
- # clojure-nl (6)
- # clojure-sanfrancisco (1)
- # clojure-seattle (1)
- # clojure-uk (21)
- # clojurescript (40)
- # core-async (82)
- # cursive (18)
- # datomic (6)
- # duct (11)
- # figwheel-main (4)
- # fulcro (26)
- # jobs-discuss (22)
- # leiningen (18)
- # off-topic (10)
- # pathom (3)
- # re-frame (5)
- # reagent (16)
- # reitit (4)
- # shadow-cljs (8)
- # specter (7)
- # sql (16)
- # tools-deps (58)
- # xtdb (30)
@dnolen when you have a moment could you take a look at https://clojure.atlassian.net/browse/TRDR-58?
I don't have any knowledge of how this stuff performs in js/is optimised by closure to judge whether the proposed change makes sense
the proposed change would be going from
(defn- read-string*
[rdr _ opts]
(loop [sb (StringBuffer.)
ch (read-char rdr)]
(case ch
nil (err/throw-eof-reading rdr :string \" sb)
\\ (recur (doto sb (.append (escape-char sb rdr)))
(read-char rdr))
\" (str sb)
(recur (doto sb (.append ch)) (read-char rdr)))))
to
(defn- read-string*
[rdr _ opts]
(loop [sb ""
ch (read-char rdr)]
(case ch
nil (err/throw-eof-reading rdr :string \" sb)
\\ (recur (str sb (escape-char sb rdr))
(read-char rdr))
\" sb
(recur (str sb ch) (read-char rdr)))))
the usage of StringBuffer there is just a port of the usage of StringBuilder in the clj impl (where it makes a massive performance difference), but I guess it may be the other way round in cljs
@bronsa that ticket should probably show throughput so that memory/throughput tradeoff is clear
cool, I won't do anything until some good analysis is done since I have absolutely no idea what the impact may be in js (nor I have a clue about how to do performance analysis in it), was just wondering if this was an "obvious" improvement to you but sounds like no
everyone OK with this?
defonce
is really a REPL affordance - I don't see how it would be meaningful in advanced
@anmonteiro so that just removes the existence check right?
@dnolen yes the only goal here is to reduce code size
and eliminate the existence check
@anmonteiro put that in the issue - does that actually cause code not to eliminated?
@dnolen yes - compare these 2 cases: https://closure-compiler.appspot.com/home#code%3D%252F%252F%2520%253D%253DClosureCompiler%253D%253D%250A%252F%252F%2520%2540compilation_level%2520ADVANCED_OPTIMIZATIONS%250A%252F%252F%2520%2540output_file_name%2520default.js%250A%252F%252F%2520%253D%253D%252FClosureCompiler%253D%253D%250A%250A%252F%252F%2520ADD%2520YOUR%2520CODE%2520HERE%250A%250A%252F**%2520%2540define%2520%257Bboolean%257D%2520*%252F%250Avar%2520DEBUG%2520%253D%2520false%253B%250A%250Aif%2520(!DEBUG%2520%257C%257C%2520x%2520%253D%253D%253D%2520undefined)%2520%257B%250A%2520%2520var%2520x%2520%253D%2520'1'%253B%250A%257D%250A%250Ay%2520%253D%2520x%2520%252B%2520%25222%2522%253B%250A%250Aif%2520(z%2520%253D%253D%253D%2520undefined)%2520%257B%250A%2520%2520var%2520z%2520%253D%2520'1'%253B%250A%257D%250A%250Ay%2520%253D%2520z%2520%252B%2520%25222%2522%253B
under advanced
Closure can inline the first case (which would be what my fix does), but not the second