hoplon

The :hoplon: ClojureScript Web Framework - http://hoplon.io/
flyboarder 2018-06-26T16:17:05.000298Z

@alandipert can we get your thoughts on something

2018-06-26T16:17:51.000384Z

sure, what's up?

flyboarder 2018-06-26T16:18:41.000216Z

@jjttjj is working on some performance improvements for 7.3

flyboarder 2018-06-26T16:19:25.000507Z

currently we are using a transient which seems to add better performance for large collections of elements

flyboarder 2018-06-26T16:20:15.000465Z

@jjttjj maybe you should explain it 😛

2018-06-26T16:20:54.000735Z

so basically these changes

flyboarder 2018-06-26T16:20:58.000555Z

^ here is the relevant changes

2018-06-26T16:22:17.000532Z

it seems like for child-vec i can add a transient and it becomes more performant after the size of the vec = 10, otherwise it's faster without, so it depends what you want to optimize for (i'm leaning towards the non-transient)

2018-06-26T16:22:54.000566Z

for vflatten i'm getting pretty tossup results with and without transient but either way both are faster than the current vflatten

2018-06-26T16:23:05.000451Z

(formatting benchmarks and results in a gist now)

2018-06-26T16:24:19.000652Z

i have to finish a mtg, but i'll look and get back to you

2018-06-26T16:24:39.000058Z

my first blush is, can we just use arrays where we use child-vec and get max perf

2018-06-26T16:24:59.000631Z

since we don't expose the children as an immutable collection anywhere

2018-06-26T16:26:36.000035Z

(sinci this is an internal state maintenance bit. and transients are helpful when you eventually need to export an immutable thing and don't want to pay the cost of copying from an array to an immutable thing)

2018-06-26T16:27:26.000292Z

needs to be associative due to the stuff like this: https://github.com/hoplon/hoplon/blob/master/src/hoplon/core.cljs#L287

2018-06-26T16:30:37.000482Z

could perhaps be (aset kids i %2) ?

flyboarder 2018-06-26T16:30:41.000552Z

@jjttjj looking at ^ I think we can make it non-associative

flyboarder 2018-06-26T16:30:54.000614Z

everywhere else its used as a vec

flyboarder 2018-06-26T16:31:11.000118Z

so I think we could use a js array in place

flyboarder 2018-06-26T16:31:32.000884Z

@alandipert thanks for the insight

2018-06-26T16:31:51.000072Z

np, fwiw i think transients are a good step, and i'm +1 on the PR

2018-06-26T16:32:01.000801Z

but i just know that the end of the perf game is writing basically JS

2018-06-26T16:32:10.000149Z

so if you have the time and interest, maybe just go that route

2018-06-26T16:32:12.000004Z

so you think it's worth basically just switching out all the swap!s for asets in core.cljs and work with js objects?

flyboarder 2018-06-26T16:32:31.000647Z

yeah basically get as close to js as possible

2018-06-26T16:33:49.000451Z

immutable -> transient -> mutable/JS -> rethink algorithm

1👍
2018-06-26T16:36:29.000373Z

yeah after a few more runs it does look like the transient version is faster, but pretty close to non-transient

2018-06-26T16:38:27.000786Z

mutating arrays directly is a lot easier on the GC also

2018-06-26T16:38:36.000298Z

especially if you eliminate destructuring/seq construction everywhere

2018-06-26T16:39:23.000702Z

when micha switched javelin to use arrays instead of transients/atoms/vectors there was a dramatic speedup and i didn't think it was much more complicated

2018-06-26T16:40:39.000709Z

so i think child-vec can be managed internally with js-objects, but vflatten will always have to work with clojure vectors, otherwise users would have to pass in js arrays for sibling elements

2018-06-26T16:44:30.000316Z

vflatten could build an array instead of a vector though, right? like its argument could be a vector but its return could be a js array

2018-06-26T16:45:14.000242Z

oh yeah true good point

2018-06-26T16:45:26.000564Z

also in add-children! it looks like vflatten is used just to make a collection to iterate over

2018-06-26T16:45:33.000892Z

could avoid creating the collection by iterating and flattening at the same time

2018-06-26T16:46:46.000244Z

i should get back to my work in my other even weirder language, R. but this is really exciting, i wish you guys luck

2018-06-26T16:46:54.000473Z

lemme know if you want me to look at anything more

flyboarder 2018-06-26T16:47:00.000177Z

thanks alan!

2018-06-26T16:47:07.000215Z

cool thanks!

2018-06-26T16:53:25.000121Z

oh, one more thing i wanted to mention

2018-06-26T16:53:48.000243Z

in that situation, add-children!, where you want to avoid an intermediate collection by walking a structure and doing something in the same loop

2018-06-26T16:54:11.000529Z

one way to do that efficiently without needing to encode in each instance the walking logic (the vflatten)

2018-06-26T16:54:22.000106Z

is a custom iteration macro, in the spirit of areduce

2018-06-26T16:55:14.000539Z

if that doesn't make sense i can elaborate. just a nugget i've picked up along the way that i thought could be helpful

2018-06-26T17:18:05.000079Z

gotcha i think i know what you mean

2018-06-26T17:18:30.000622Z

actually discovered areduce for the first time when looking at improving child-vec

2018-06-26T17:18:51.000545Z

FWIW my benchmark code before had a dumb bug that made the results totally wrong

2018-06-26T17:19:02.000430Z

this is updated with an array version

2018-06-26T17:20:03.000341Z

also shows how close a call the transient and non-transient version are which was my original point, that i messed explaining due to that benchmark bug:man-facepalming:

flyboarder 2018-06-26T18:01:42.000018Z

@jjttjj nice work on the benchmarking

flyboarder 2018-06-26T18:19:40.000470Z

https://twitter.com/degree9io/status/1011675229683236864

2018-06-26T22:10:43.000058Z

hi there @smnplk

2018-06-26T22:31:37.000395Z

After taking an initial crack at using js arrays instead of vector to store the hoplon-kids i have a few more thoughts. I didn't get it quite working yet but just started work on it. I got a sense that this is gonna be a pretty clear tradeoff, getting performance in exchange for giving up a lot of clojure niceties in hoplon.core Is performance an immediate priority for hoplon? It seems like this would be a pretty big change in terms of amount of code in core that is effected. Not to mention there should probably be some benchmarks/performance measurements set up first to actually gauge how much this improves things, and to see where the bottlenecks are in the first place? To be honest, hoplon's performance has never really been an issue for me. With the two changes in the PR I was looking for really low hanging fruit things that could be refactored as my main priority, and get speed gains where possible (these managed to get ~35%+ speedup, and cleaned up the code with no tradeoffs :)) The re-working of the innards to use JS is an interesting project I think it deserves more research and thought but it doesn't seem obvious to me that this would be a great fit for 7.3 in the immediate term? I guess having some sense of the goals of hoplon now would be useful. Any thoughts @alandipert @flyboarder

flyboarder 2018-06-26T22:33:29.000013Z

@jjttjj im all for pushing the move to js arrays off to another version, the improvements to the core loops should be enough for now to combat any performance criticism

flyboarder 2018-06-26T22:34:32.000162Z

also this already cleans up all the remaining old code from the v6-alpha’s

flyboarder 2018-06-26T22:35:02.000412Z

with the exception being the loop-tpl*

flyboarder 2018-06-26T22:35:21.000065Z

that could be greatly simplified from it’s current form

2018-06-26T22:36:45.000399Z

yeah I looked at that issue you sent me regarding that, the new version there looks pretty good initially, will play around with that too a bit extensively soon

2018-06-26T22:37:09.000504Z

i think it's a really good idea to hit a conceptual clarity milestone before a major perf/JS excursion

2018-06-26T22:37:29.000263Z

like javelin tho i have the sense hoplon is a good place to mine for performance, since we're happy with what it does

2018-06-26T22:37:35.000747Z

and so can safely play around with how it does it

2018-06-26T22:38:07.000003Z

so, excellent points @jjttjj

2018-06-26T22:38:33.000559Z

here's my very initial, not working naive attempt at using js kids https://gist.github.com/jjttjj/64933897fd45985a3642980fda57fac5

2018-06-26T22:39:53.000479Z

actually I just realized amap exists which would clean up some of those reduces

2018-06-26T22:40:41.000016Z

i could see it being done cleanly eventually just kind of a big overhaul

flyboarder 2018-06-26T22:44:42.000380Z

@jjttjj well lets skip the js arrays for now, I’ll open a ticket and set it for 7.5

2018-06-26T22:50:53.000478Z

cool so for now i guess the question is back to:

(defn- vflatten
  "Takes a sequential collection and returns a flattened vector of any nested
  sequential collections."
  ([x] (vflatten [] x))
  ([acc x] (if (sequential? x) (reduce vflatten acc x) (conj acc x))))
vs
(defn- vflatten
  "Takes a sequential collection and returns a flattened vector of any nested
  sequential collections."
  ([tree]
   (persistent! (vflatten (transient []) tree)))
  ([acc tree]
   (if (sequential? tree)
     (reduce vflatten acc tree)
     (conj! acc tree))))
The NON transient wins for smaller, flatter values https://gist.github.com/jjttjj/b9ab914009e360f7f765782ef7462bb1

2018-06-26T22:53:38.000086Z

i have a similar benchmark for child-vec with vs without transient, but there i think it's even more clear the non-transient wins

2018-06-26T22:54:50.000484Z

it all depends what params we want to optimize for though, I have a feeling smaller, flatter children vectors are the vast majority