This page is not created by, affiliated with, or supported by Slack Technologies, Inc.
2017-06-29
Channels
- # aws (6)
- # beginners (33)
- # bitcoin (2)
- # boot (22)
- # carry (2)
- # cider (5)
- # clara (21)
- # cljs-dev (115)
- # cljsrn (40)
- # clojure (161)
- # clojure-dev (73)
- # clojure-italy (38)
- # clojure-russia (88)
- # clojure-spec (123)
- # clojure-uk (58)
- # clojurescript (88)
- # core-async (26)
- # cursive (5)
- # datascript (18)
- # datomic (26)
- # hoplon (50)
- # java (2)
- # jobs (1)
- # leiningen (10)
- # lumo (1)
- # off-topic (18)
- # om (9)
- # onyx (26)
- # parinfer (13)
- # pedestal (41)
- # quil (1)
- # re-frame (27)
- # reagent (21)
- # ring-swagger (11)
- # slack-help (3)
- # spacemacs (8)
- # specter (5)
- # sql (42)
- # timbre (1)
- # uncomplicate (7)
- # untangled (3)
- # videos (1)
- # yada (26)
I can'f find anything in the old tickets about the history of -cljs$lang$maxFixedArity
, is there a reason CLJS doesn't just use f.cljs$core$IFn$_invoke$arity$variadic.length - 1
instead?
@bronsa Yes, it'll be set to 4 by CLJS right now. And it'd be 4 with using variadic - 1 too.
I'm just brainstorming how to centralize all dispatchers & applyto + .call
+ .apply
. I'm pretty sure we can get away with removing everything on multi arity functions except the actual implementations.
@rauh before going down that rabbit hole I would spend some time researching performance implications
I suspect the primary benefit of doing this would be code size - not performance which I’m pretty confident will degrade significantly
while I won’t say don’t look into this - I would argue that there are performance gains to be had that don’t require so much research or vetting
@dnolen So you mentioned refactoring IFn
invokes a few weeks ago. Is https://dev.clojure.org/jira/browse/CLJS-2127 too many changes? Should I just introduce new fn's and use them only in --let's say-- apply
for now? I kinda wanna get those in before starting the other work.
@dnolen I disagree, the single dispatcher is never used really. It just does't run under "normal" code in advanced
basically the first thing I want to see is (invoke* ...)
and to just use that consistently everywhere
well yeah behavior is the first thing to check, but I meant more implementation details abou the seq types
Ok, so digging into the clojure implementation it also seems to call next on the args. The two sequences involved with PersistentHashMap NodeSeq and ArraySeq also seem to have a next method
Should all implementors of ISeq implement INext? it looks like in Clojure next
is really part of ISeq
for(ISeq s = RT.seq(args); s != null; s = s.next())
seems to confirm that is an assumption
@tmulvaney next
appeared early on in Clojure so wasn’t a big deal, pre deftype
so when I added them for perf I didn’t fold into an existing protocol for backward-compatibility
and nothing we can do about that - except make sure INext is on all the existing ISeq iplementations
ok cool. Well it looks like its just those ArrayNodeSeq and NodeSeq, as well as PersistentQueue seq which are missing it
Thanks for the back story. It's always good to know the how and why. I've attached a patch at https://dev.clojure.org/jira/browse/CLJS-2137
@tmulvaney patch looks ok but why did you change the test code?
I mentioned in the comment that there was an undeclared variable assoc-n
@favila pointed it out to me yesterday
@tmulvaney can you update the patch to leave a comment there please
@tmulvaney looks like your patch needs a rebase?
@tmulvaney you’re right something weird locally
@rauh that’s interesting but not really all that useful since we don’t prioritize V8 at all
we also need constantly counterbalance w/ SpiderMonkey which continues to lag on the perf front
Personally I don't care about desktop/laptop performance. Everything will be super fast anyways. My only problem is mobile. And in that case Apple products are pretty darn fast b/c of their processors. And given Android is much more spread on third world countries which is a HUGE market and often have slower phones. Hence why IMO Android Chrome/v8 needs most attention.
Also, the hidden class thing is the same on almost all browser engines. So I'll see if setting IFn
protocols up to 10 args simply to nil
in the centralized dispatcher could lead to all multiy arity function getting the same hidden class.
we don’t care about V8 in this way and will never spend any effort optimizing specifically for it
Yeah, I'm not saying I want to specifically target any browser. It's just the easiest to develop with. The v8 command gives some easy and quick insights
Just wanted to share, in case somebody wants to play around. I'll come up with real benchmarks once I implement the central dispatchers idea.
@dnolen I noticed a bit of dead code whilst writing the last patch, which I've addressed in CLJS-2138
@dnolen Re: Refactoring: So you mean add helper fn (not macro)? I kinda really need a macro to invoke the $variadic$
or it'll get ugly. Can I add an invoke-variadic*
macro?
Right, but I'm realizing that I'd need a helper macro for invoking the variadic property of a fn. Or I'd have to repeat (.cljs$core$IFn$_invoke$arity$variadic f (seq args))
many times.
won’t have time to look at this today or tomorrow but this is a pretty bad unresolved warning regression - https://dev.clojure.org/jira/browse/CLJS-2139