@rauh thanks! so it just sees if any arg is a constant to optimally select the appropriate comparison function, and falls back to original function
is this doable as a core patch?
An experiment to see if filter can be made to run faster if the predicate is a Var tagged ^boolean (using unchecked if): https://gist.github.com/mfikes/7a725048ce6eb92b6965393e913558b1
@mfikes interesting though the perf benefit is surprisingly small
Yep. Probably not worth pursuing further. Just sharing.
@rauh that version of = is probably not a good idea, you really can’t swap in keyword-identical? like that w/o introducing a breaking change and a lot of confusion
actually, sorry I was confused - was thinking about somebody having proposed this for identical?
@shaunlebron inlining = like @rauh has done is not a bad idea and would cover the Closure define case, patch welcome
perhaps my only suggestion would be to cover a few more cases like nil, true, false
hrm on a second thought doing this means breaking = extension yourself for these values
We could straight up call IEquiv if one of them is a coll? like (= [0] the_exp) -> (.cljs$core$IEquiv$_equiv$arity$2 [0] nil the_exp) (possibly adding ^not-native)
Also could do symbol-identical?. Though I think most common cases are covered by the above code.
@rauh what I mean is that inlining prevents would-be extenders
= is intentionally extensible
Right, that's impossible to do then... And those extender folks would also rely on the fact that = function right now calls (-equiv x y), so IEquiv on the first argument (and not on the second argument). So we could change the above by only checking the first argument x for keyword?/`number?` (etc). and emit specialized code.
But not if y is a keyword, since the custom type could potentially define its own IEquiv.
So the question is if we wanted to optimize that then... Or just leave it as it is.
leave as is since it would be a big breaking change and has no effect on existing highly optimized code
so the breaking case is that someone overrides -equiv for keywords/booleans/strings?
(extend-type Keyword IEquiv (-equiv [o other] false))
worth mentioning that this type of overriding is ignored when comparing constants when optimize-constants is true, since = will first check identical?
perhaps that means we can still safely perform this optimization when comparing a goog-define to a constant (which is the motivating use-case here)
(i.e. a goog-define is limited to a string/number/boolean, so comparison to a constant would have the same current behavior as = due to the identical? shortcut)
@shaunlebron it’s not ignored
if identical? is false then we dispatch
hmm, if we have access to the goog-define constant at compile-time, we can still expand it to identical and ignore the dispatch case since we know they are equivalent
I think we should table any notion of messing around with base semantics of =
@shaunlebron yes, something less invasive like that is preferred
just special case goog-define vars - you’ll have declared their values for :advanced so we’ll know
k thanks, i’ll try patching
thanks
@shaunlebron I don't think you'd need the type hint in this case. I've been running with macro like this and it's working well:
(core/defmacro =
([x] true)
([x y]
(core/cond
(core/or (core/keyword? x) (core/keyword? y))
(bool-expr
(core/list 'cljs.core/keyword-identical? x y))
(core/or (core/string? x) (core/string? y) (core/char? x) (core/char? y))
(bool-expr
(core/list 'cljs.core/identical? x y))
(core/or (core/number? x) (core/number? y))
(bool-expr
(core/list 'cljs.core/== x y))
:else
(bool-expr
(core/list 'js* "cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(~{},~{})" x y))))
([x y & xs]
(bool-expr
(core/list 'js*
"cljs.core._EQ_.cljs$core$IFn$_invoke$arity$variadic(~{}, ~{}, ~{})"
x y `(cljs.core/array-seq (cljs.core/array ~@xs) 0)))))
(core/defmacro not=
([x] false)
([x y] `(coercive-not (= ~x ~y)))
([x y & xs]
`(coercive-not (= ~x ~y ~@xs))))