same get be done with dissoc

You can also use the inliner to opportunistically unroll get-in when the vector is written literally at the call site, which is very often the case

the speedups for get-in and for select-keys are very significant https://github.com/bsless/clj-fast/blob/master/doc/results.md#get-in https://github.com/bsless/clj-fast/blob/master/doc/results.md#select-keys

@ben.sless I think a lot of these things could be linting rules in clj-kondo as well

Optional of course

The problem here is that optimizing is usually only relevant in certain contexts and not globally

for the linter I mean

Yep. Also, I can already guess what Rich will say when he looks at such a patch: it increases the amount of generated bytecode, this will mess with JITing, hard pass.

I can see some linting rules around this being useful when applied namespace-locally

and this is easily analyzed statically:

(get-in foo [:foo :bar])

(clj-kondo already has the ability to turn on rules namespace-locally)

Consider here that's also an issue with emitted bytecode size. (:foo m) generates more code than (get m :foo), and after the JIT really kicks in the difference is quite small

Also, funny business:

(def m {:a 1})
(with-out-str (time (dotimes [_ 100000000] (get m :a))))
;; => "\"Elapsed time: 1032.999081 msecs\"\n"
(with-out-str (time (dotimes [_ 100000000] (:a m))))
;; => "\"Elapsed time: 1793.317199 msecs\"\n"

would it be good to say that like with assoc, it's more optimal to replace get-in with nested get when the length of the path is statically known?

absolutely

but again, it may have unexpected effects on performance, so I'd want to hear some of the core devs' opinion on this

user=> (time (dotimes [i 10000000] (get-in a [:a :b])))
"Elapsed time: 384.070726 msecs"
nil
user=> (time (dotimes [i 10000000] (get (get a :a) :b)))
"Elapsed time: 153.975087 msecs"
nil

the problem with linter advice like "Replace get-in by -> for better performance" is that this advice may be true for some clojure/java versions but gets cargo culted even when it stops being true.

and usually it doesn't matter anyways

so people focus on the wrong things

I don't disagree with that :)

Happy to run this against our codebase. How do I install grasp on macOS? I looked in the repo and didn't see a binary to download?

@U04V70XH6 Just run this on the JVM: https://gist.github.com/borkdude/e6f0b12f9352f3375e5f3277d2aba6c9

invoke as bash script

Yup, just spotted that and ran it. Thanks!

and from ours:

([1 1708] [2 287] [3 67] [4 25] [5 5] [7 3] [8 2] [13 1] [9 1])

29?!

haha

This is probably where a transient may start to pay off?

Mine in 100kLOC codebase

([1 923] [2 123] [4 38] [5 35] [3 34] [6 32] [7 25] [8 6] [11 2] [12 1] [14 1])

Results from our 200k LOC codebase:

Total: 15776
 1: ******************************************************************************* (12330, 79%)
 2: **************** (2425, 16%)
 3: ***** (642, 5%)
 4: ** (223, 2%)
 5: * (114, 1%)
 6: * (22, 1%)
 9: * (6, 1%)
 7: * (3, 1%)
11: * (3, 1%)
10: * (3, 1%)
12: * (2, 1%)
20: * (1, 1%)
14: * (1, 1%)
 8: * (1, 1%)

Fixed the percentages now:

$ ~/Dropbox/temp/assoc_pairs.clj
Total: 1002
 1: ********************************************************************************* (814, 81,24%)
 2: ************ (120, 11,98%)
 3: ***** (55, 5,49%)
 5: * (6, 0,60%)
 4: * (3, 0,30%)
 9: * (2, 0,20%)
 6: * (2, 0,20%)

I even wrote some code that unrolls a static map merge to a series of assoc-s

True, but it happens more often than you'd think

I could argue that if your associng a bunch of things, then maybe you’re missing a semantic thingy in your program.

i don't follow. assoc'ing a few things seems like its using one of the basic building blocks of clojure

Sorry, I was thinking higher up in the stack, like in the business domain. If you’re assoc’ing a bunch of things on to another thing, then maybe those other things together are a thing in your domain.

for dir in work-projects/* ; do ./grasp $dir "(g/seq 'assoc any? any? any? (s/+ any?))" | grep file | wc -l | grep -v 0 ; done

This is the number of times usage of the rest arity of assoc was found in projects

I can report I tested unrolled versions of assoc for up to 4 keys for different map sizes and saw speedups for all of them

I kicked it off a couple of hours ago on my entire "workspace" folder which has both our work codebase (that I reported on earlier) and every single OSS project I work on. It's still running 🙂

I think I want to include a couple of hooks so you can say: if this file doesn't contain "assoc" at all, just skip it. And, if this form doesn't contain "assoc" at all, just skip it as well. I'm trying this now locally on my .m2 folder...

the resulting forms from grasp are returned as a lazy-seq so you can already build some rolling average reporting on top of it

There may also be a bug in grasp currently, I'm looking into it

Why it's not progressing after a certain amount of time

I think I found something using visualVM: there might be a bug in the parser

OK, I'll kill my process (that was still running 3 1/2 hours later).

Yeah, that's probably best. I had a bug in the parser before, but that was already fixed. It seems clojure was using and older version of the parsing lib although the deps.edn of the gitlib was already updated. It seemed to work again with -Sforce but now running into something else. I'll report back when I can "grasp" my entire m2.

(found it, it's a bug with parsing .cljc from data.xml - node.cljc to be specific)

(in case you wanted to know what the bug was: the parser didn't expect there could be whitespace between a reader conditional splice and the next expression: #?@\n(:clj ..))

(like here: https://github.com/clojure/data.xml/blob/97c715acb339ac3e20269620b3fccc480bbd9722/src/main/clojure/clojure/data/xml/node.cljc#L46)

I just grep'd all the code I have here and there are two occurrences in that one file and none in any of the other source code I have checked out.

ok, well, glad it's fixed now.

Maybe I should grab a clojars dump and hook up the analysis I've been threatening for years.