Fork me on GitHub
#cljs-dev
<
2017-04-06
>
thheller19:04:59

@dnolen @favila @rauh to pick on the str issue, there is a measurable gain in the solution @rauh proposed

thheller19:04:17

current impl is 27% slower in chrome, 73% slower in safari, 80% slower in firefox (on my machine, macbook, no IE)

rauh19:04:10

45% chrome, 80% FF here

rauh19:04:29

IMO, it's such a simple change that makes sense even just for the sake of allowing GCC to pre-concat strings and not emitting code like str("some str").

rauh19:04:35

It decreased the js code for me by 1.5%, which seems tiny, but a few of those changes... And it adds up.

rauh19:04:11

Here is another one: In destructure if, instead of checking if the passed val is a map/seq inline (as it's done right now) and instead create a function like ensure-map we'd decrease size + be about 10% (FF) 30%(Chrome) faster. Significant? No, but a small improvement. (all tested in advanced builds)

favila19:04:10

Would there be an even bigger benefit to getting rid of macro-generated [].join() alltogether? (using +)

favila19:04:56

This was my original approach: CLJS-801, but was wrongly implicated in the Safari x.toString() debacle and backed out

rauh19:04:05

I found that [].join is much faster in FF than +. Not so in chrome.

favila19:04:34

even for constants?

favila19:04:42

rather, for literals

rauh19:04:17

Well we shouldn't join constants, that's argument #1 🙂

favila20:04:05

I mean a+b+c+d+e+f (where all args are known) vs appending to a string in a loop

favila20:04:26

the former is the macro case, the later the str function case with many args

favila20:04:04

well, StringBuffer uses +=

thheller20:04:06

added that test as well

thheller20:04:26

but given the history of invalid results we should be careful with that I think

thheller20:04:31

it it faster though

thheller20:04:14

just noticed an error

thheller20:04:14

https://jsperf.com/cljs-str-perf fixed the error, please re-check results

favila20:04:59

FF still looks faster with + to me

rauh20:04:32

Yeah, same. I wonder what I measured the other day 🙂

rauh20:04:39

@thheller Why did you remove the testcase where the str function just returned x instead of [x].join("")?

favila20:04:31

that gives incorrect results with + on the outside

thheller20:04:39

I only want to change one thing

favila20:04:32

Unfortunately due to dropbox killing public folders I can't get that jsperf working again

favila20:04:24

but the msg reminded me that ''+[x] is also an alternative to [x].join("") to avoid x.toString()

rauh20:04:03

I'm still getting FF being much faster with join than for +

thheller20:04:40

yeah results are mostly unchanged from before, just wanted to be correct. + is faster by a lot in every engine

spinningtopsofdoom20:04:55

Does Firefox use ropes for strings like the other browsers? That might be the reason join is faster

favila20:04:50

It should. It did two years ago last I tested. And honestly it's still faster now on my machine. @rauh what FF version and platform?

favila20:04:34

Using FF 52 on ubuntu I see + as 33% faster than join

rauh20:04:57

Same version + Platform

favila20:04:14

32 or 64 bit?

rauh20:04:33

but with a version of joining with a function that returns x if x is not null

rauh20:04:02

so: function(x) { (x==null)?"":x}

rauh20:04:13

well obviously forgot the return here... Too much clojure 🙂

favila20:04:42

found the old code for that jsperf that doesn't run

favila20:04:49

These were the str impls I tried 2 years ago

favila20:04:02

(str fn, not macro)

rauh20:04:11

@favila Here is the test case that's faster in ff:

favila20:04:41

man that's weird

favila20:04:20

also weird, the fastest FF case is still slower than the slowest Chrome case

favila20:04:50

"inlined str +" is 5x on Chrome

thheller20:04:49

micro-benchmarks like this are always a bit dangerous

thheller20:04:00

but given that we all see similar results there is something to them