@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)))))

(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)))))

i.e. doing away with StringBuffer in favour of just using str

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

but also maybe they should try transit

also the ticket doesn't show memory usage for a variety of JS engines

I wouldn't proceed w/ that ticket w/o a lot more information

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

I'll follow up with the original reporter then, thanks

bump on https://clojurians.slack.com/archives/C07UQ678E/p1562706806320500

everyone OK with this?

I don't have any issues with that

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?

one question I have is what problem that actually fixes in your case?

@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