Fork me on GitHub
#cljs-dev
<
2017-05-15
>
rauh19:05:34

Added comment to https://dev.clojure.org/jira/browse/CLJS-1871 a few days ago (self calls aren't optimized). This is pretty different to the original ticket. 1. Should this go into a separate ticket? 2. Most performance is for TreeMap. reduce-kv is about 2-3x faster (Chrome,FF) with proper calls, dissoc about 1.5-1.8x faster. 3. There is also: https://dev.clojure.org/jira/browse/CLJS-275 . Is this a regression?

rohit19:05:24

@rauh: is this true even under advanced compilation?

rauh19:05:42

@anmonteiro Thanks for the patches of the last few issues!

dnolen19:05:56

@rauh a regression if that isn’t true under :static-fns compiler option

dnolen19:05:23

@rauh and yes recursive calls should be considered separately

rauh19:05:45

:static-fns is default for advanced, right?

dnolen19:05:10

that’s correct, but there’s no need to bother with advanced when checking for this

thheller19:05:06

@rauh there is also https://dev.clojure.org/jira/browse/CLJS-1992 which messes with some method calls, don't remember if maybe one of yours is affected by this.

rauh19:05:54

@thheller Yeah I saw those. Good catch!

rauh19:05:21

So I'm thinking this is a regression then. I'm seeing this in advanced+pseudo build:

var $cljs$core$tree_map_kv_reduce$$ = function $cljs$core$tree_map_kv_reduce$$($node$jscomp$35$$, $f$jscomp$244$$, $init$jscomp$14$$) {
  var $init__$1$jscomp$8$$ = null != $node$jscomp$35$$.left ? function() {
    var $init__$1$jscomp$8$$ = $node$jscomp$35$$.left;
    return $cljs$core$tree_map_kv_reduce$$.$cljs$core$IFn$_invoke$arity$3$ ? $cljs$core$tree_map_kv_reduce$$.$cljs$core$IFn$_invoke$arity$3$($init__$1$jscomp$8$$, $f$jscomp$244$$, $init$jscomp$14$$) : $cljs$core$tree_map_kv_reduce$$.call(null, $init__$1$jscomp$8$$, $f$jscomp$244$$, $init$jscomp$14$$);
  }() : $init$jscomp$14$$;
  if ($cljs$core$reduced_QMARK_$$($init__$1$jscomp$8$$)) {
    return $cljs$core$_deref$$($init__$1$jscomp$8$$);
  }
  var $init__$2$jscomp$9$$ = function() {
    var $init$jscomp$14$$ = $node$jscomp$35$$.key, $init__$2$jscomp$9$$ = $node$jscomp$35$$.$val$;
    return $f$jscomp$244$$.$cljs$core$IFn$_invoke$arity$3$ ? $f$jscomp$244$$.$cljs$core$IFn$_invoke$arity$3$($init__$1$jscomp$8$$, $init$jscomp$14$$, $init__$2$jscomp$9$$) : $f$jscomp$244$$.call(null, $init__$1$jscomp$8$$, $init$jscomp$14$$, $init__$2$jscomp$9$$);
  }();
  if ($cljs$core$reduced_QMARK_$$($init__$2$jscomp$9$$)) {
    return $cljs$core$_deref$$($init__$2$jscomp$9$$);
  }
  var $init__$3$jscomp$2$$ = null != $node$jscomp$35$$.right ? function() {
    var $init$jscomp$14$$ = $node$jscomp$35$$.right;
    return $cljs$core$tree_map_kv_reduce$$.$cljs$core$IFn$_invoke$arity$3$ ? $cljs$core$tree_map_kv_reduce$$.$cljs$core$IFn$_invoke$arity$3$($init$jscomp$14$$, $f$jscomp$244$$, $init__$2$jscomp$9$$) : $cljs$core$tree_map_kv_reduce$$.call(null, $init$jscomp$14$$, $f$jscomp$244$$, $init__$2$jscomp$9$$);
  }() : $init__$2$jscomp$9$$;
  return $cljs$core$reduced_QMARK_$$($init__$3$jscomp$2$$) ? $cljs$core$_deref$$($init__$3$jscomp$2$$) : $init__$3$jscomp$2$$;
};

rohit20:05:07

@rauh: So I see what you mean:

rohit20:05:09

(declare helper)

(defn redirect-helper
  [a b]
  (helper a b))

(defn helper
  [a b]
  (js/console.log a b))

rohit20:05:21

produces:

rohit20:05:23

testapp.core.redirect_helper = (function testapp$core$redirect_helper(a,b){
return (testapp.core.helper.cljs$core$IFn$_invoke$arity$2 ? testapp.core.helper.cljs$core$IFn$_invoke$arity$2(a,b) : testapp.core.helper.call(null,a,b));
});
testapp.core.helper = (function testapp$core$helper(a,b){
return console.log(a,b);
});

rohit20:05:42

This is with {:static-fns true}

dnolen20:05:15

hrm this maybe less a regression then it never working on self calls of fns with only one arity implementation

rauh20:05:11

Yeah there is another ticket where this was specifically optimized for multi-arity fns... But I can't find it anymore 😕

dnolen20:05:14

in anycase, it’s a significant optimization to make it work for this case, so if you want to give it a try go for it - I suspect it may be a simple mistake somewhere