Clojurians Log v2

(defn conj-conj!
  "conj! that matches the behavior of conj in terms of handling transients."
  ;TODO measure performance cost
  ([] (transient []))
  ([coll] coll)
  ([^clojure.lang.ITransientCollection coll x]
   (if (contains? coll x)
     coll
     (.conj coll x))))

;; conj! an item with metadata vs regular conj problem showcase 
(comment
  (let [set-debug (fn [s msg]
                    (set! *print-meta* true)
                    (println msg)
                    (clojure.pprint/pprint
                      {:item-from-set      (s [:a])
                       :first-of-set       (first s)
                       :seq-of-set         (seq s)
                       :meta-of-first-of-s (meta (first s))})
                    (println))
        v1        (with-meta [:a] {:n 0})
        v2        (with-meta [:a] {:n -42})
        ;conj! problem
        s         (-> (transient #{}) (conj! v1) (conj! v2))
        bad-set   (persistent! s)
        ;conj-conj! fix
        s         (-> (transient #{}) (conj-conj! v1) (conj-conj! v2))
        good-set  (persistent! s)]
    (set-debug bad-set "bad set")
    (set-debug good-set "good set, maybe?")))

bad set
{:item-from-set ^{:n -42} [:a],
 :first-of-set ^{:n 0} [:a],
 :seq-of-set (^{:n 0} [:a]),
 :meta-of-first-of-s {:n 0}}

good set, maybe?
{:item-from-set ^{:n 0} [:a],
 :first-of-set ^{:n 0} [:a],
 :seq-of-set (^{:n 0} [:a]),
 :meta-of-first-of-s {:n 0}}

Seems to “work”; the biggest unknown is the performance cost of the “read before every transient conj! “; i.e., the worst-case scenario is if this performs same/worse than regular conj on a #{} ; In any case, it should generate less garbage collector pressure, as far as my understanding of the immutable persistent data structure goes. Whether that’s significant depends on… many things.

Previously discussed, and existing issue: https://clojure.atlassian.net/issues/CLJ-1615 https://clojurians.slack.com/archives/C03S1KBA2/p1744219939763269

(if (contains? coll x) For sets there's a couple different ways to do this if performance does end up being poor (e.g. (get coll x) , (coll x), etc.) wouldn't expect performance to be that notably different but probably worth benchmarking them all

Sets can contain nil so (get coll x) is not the same as (contains? coll x)

Also, (get coll false) will be false (present) or nil (not present) but (contains? coll false) will be true (present) or false (not present).

@seancorfield yeah good call

Would there be any benefit to calling (. clojure.lang.RT (contains coll key)) directly or does that not matter due to contains? being marked :static in its metadata?

Because of stuff like JIT optimization, the only real way to tell for these sorts of minor performance tweaks is to benchmark them with a wide range of inputs. There's also the question of whether you want to address this for JVM Clojure only or also for ClojureScript et al.

@seancorfield for sure; my main motivation was to match the behavior of conj which seems to work on the surface; Performance is a more tricky topic with a ton of context specifics, I agree.

@emma.griffin as far as I know, invoke on a coll (#{:a} :a) can be typically ever-so-slightly faster than get . Probably a few nanoseconds?

In my experience though, that’s probably the very last place to look for performance gains… Chasing those is almost certainly a red herring, and I’ve been down those rabbit holes at times for an hour or two.

https://github.com/clojure/clojure/commit/838302612551ef6a50a8adbdb9708cb1362b0898

looks like it was a really fast slot. and then perhaps the idea was the most pervasive slots we have like this are mapentries and so seems like vector just impelements that?

❯ clj -Sdeps '{:deps {org.clojure/clojure {:mvn/version "1.8.0-RC3"}}}'
Downloading: org/clojure/clojure/1.8.0-RC3/clojure-1.8.0-RC3.pom from central
Downloading: org/clojure/clojure/1.8.0-RC3/clojure-1.8.0-RC3.jar from central
Clojure 1.8.0-RC3
user=> (.getKey [:a :b])
:a
user=> (key [:a :b])
:a

❯ clj
Clojure 1.12.0
user=> (.getKey [:a :b])
Execution error (IllegalArgumentException) at user/eval1 (REPL:1).
No matching field found: getKey for class clojure.lang.PersistentVector
user=> (key [:a :b])
Execution error (ClassCastException) at user/eval3 (REPL:1).
class clojure.lang.PersistentVector cannot be cast to class java.util.Map$Entry (clojure.lang.PersistentVector is in unnamed module of loader 'app'; java.util.Map$Entry is in module java.base of loader 'bootstrap')
user=>

There might be contemporaneous conversation still on another forum. The comment at the top says "/ rich 7/16/15 / // proposed by Zach Tellman" reminds me of a time when Zach had demonstrated generating tuple or array classes of various sizes (for speed) and Rich, crediting it, had tried more of a one-size-fits-all and eventually concluded that either way was too big a risk of pettifogging call sites and basically the JIT technology of the time was not ripe for such things.

i saw the commit was 10 years ago so i was checking clojure versions 1.2 and didn’t think that it was in a 1.8 beta 😞

oh yeah. zach has some cool libraries. one about tuples which seems to be direct lineage here. and another about different wrapper classes that you can set values in

The only references to it in the code are in comments: https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/LazilyPersistentVector.java#L21-L40

This was an idea Zach tellman had to make fixed size small vectors for optimized perf. Rich did his version of it and while it had a lot of benefits it also made every vector callsite polymorphic instead of monomorphic which foils a lot of hot spot optimization, so Rich rolled it back

Looked great in simple benchmarks but very murky in reality

That said, that story changes a lot over time

Thanks for the explanation!

Java has two sized specialized list impls, List12 and ListN

Clojure's Tuple had like 6... Hotspot only optimizes for at maximum 2 targets

user=> (class (java.util.List/of 1))
java.util.ImmutableCollections$List12
user=> (class (java.util.List/of 1 2 3 4))
java.util.ImmutableCollections$ListN

Thanks @ghadi! Any good reading on optimizing collection implementations for hotspot?

Write it the simplistic way and move on

having 1 impl is (in general) better than having many specialized ones (see channels in core.async. There's just one)