clj-kondo 2021-04-29 | Slack Archive

Yehonathan Sharvit14:04:14

A linter idea: When a defmulti is defined as a keyword, warns in case the arity of one of the methods is greater than 1. Otherwise keyword-as-a-function treats the second argument as a default value. This mistake in my code caused an out of memory error due to the caching mechasim of multi-methods. https://ask.clojure.org/index.php/10532/memory-leak-using-the-default-method-of-a-multimethod

borkdude14:04:38

Thanks. It's a pretty niche problem I think, but I'll keep it in mind

Yehonathan Sharvit16:04:43

very niche but when it happened it cost me 5 days of painful of investigation with all kind of JVM profiling tools

Darin Douglass14:04:08

speaking of: thoughts on having a linter that checks that a defmulti dispatch fn is a var? if you provide a fn, trying to later change that fn in a repl is tricky

borkdude15:04:39

also sounds pretty niche :)

borkdude15:04:00

do you mean: give a warning so you can remove it before going into production?

Darin Douglass15:04:36

i guess you could consider it niche, though i suspect it's good practice everywhere defmultis are used

Darin Douglass15:04:07

it'd definitely be a warning, though in production theoretically fn dispatch values are ok since you shouldn't be mucking around in a repl in production 😛

borkdude15:04:16

I don't understand which of the two you find good practice

Darin Douglass15:04:36

;; this is better than ...
(defn dispatch
  [args]
  <some logic>)

(defmulti my-multi #'dispatch)

;; ... this
(defmulti my-multi (fn [args] <some logic>))

borkdude15:04:35

and you want clj-kondo to warn on the latter? I don't think I agree with this, but maybe I just haven't run into trouble enough with re-defining multi-methods. In my experience this is pretty rare.

Darin Douglass15:04:13

i think it would be useful, we've been hit a couple of times by it. the solution is to just reload your repl, but still it's annoying

didibus15:04:16

You don't need to reload the REPL

didibus15:04:08

Just do: (def my-multi nil) and then re-evaluate the defmulti and it's going to work

Darin Douglass15:04:29

oh interesting, i hadn't thought of that

Darin Douglass15:04:12

i'll have to rethink my stance then (though i suspect i'll still lean towards the "use a var" side)

didibus15:04:06

To be honest, I don't know why defmulti behaves as it does, but for some reason they went above and beyond to prevent it from being redefined and I wonder why, like is there a good reason

didibus15:04:56

What I tend to do is this:

(def my-multi nil)
(defmulti my-multi (fn [...] ...))

That way when I reload the namespace in the REPL it reloads the defmulti as well in case I changed the dispatch-fn. But in prod I'm not messing with the standard defmulti in case there's a reason it is how it is.

jeroenvandijk10:05:11

@U0K064KQV The reason you don’t want to redefine multimethods is because this would remove all extensions in other namespaces potentially. For this same reason (def my-multi nil) is not (always) a good idea. I’m using the trick of @UGRJKK74Y as well. I don’t use defmulti in performance heavy areas normally so (defmulti my-multi #'dispatch) is a better approach for my workflow

didibus10:05:52

I saw that, normally actually I have it in a comment block above and I just know to evaluate it in my REPL after I changed my dispatch FN

didibus10:05:33

Most of the time, when I change my dispatch-fn, it's because it didn't work and I don't mind re-evaluating the defmethods afterwards

didibus10:05:12

But I see the advantage of Var indirection over it, appart for performance, seems you get all the benefits and none of the negatives

➕ 3

jeroenvandijk10:05:31

yeah for me it was the best approach. I have a project where I have 5 or more namespaces extending one multimethod. So this means i have to reload those 5 namespaces. I have had long debugging sessions because of this. So I decided to always go for the var approach

jeroenvandijk10:05:30

Maybe the idea of having a macro that would remove the indirection in production could be useful here. To remove the performance argument

borkdude15:04:03

Originally I thought you were arguing the opposite: don't use a var because this is worse for performance :)

borkdude15:04:30

I guess you could even write (when dev? #'foo foo).

Darin Douglass15:04:54

Ah yah, rereading what I typed I can see the confusion, sorry bout that :P

2021-04-29

Channels