Long term performance / design considerations

[bsless]

I'm creating an issue to track this - in my benchmarks of atmorpheric-lut I reached a point where map access dominated a significant portion of CPU cycles.
Transitioning to records with primitive fields will give a speedup in access and in eliding boxing.
It will require more significant changes to the code base so I'm not going swinging in with a hammer, but I wanted to point it out at this stage.

[wedesoft]

Thanks a lot for your work so far. I am ok with changes from maps to records for performance improvements. Btw, I wonder what you think of the quaternion implementation in terms of performance.

[bsless]

Quaternions - you're boxing all over the place.
How bad is it? behold:

(defn +
  "Add two quaternions"
  {:malli/schema [:=> [:cat quaternion quaternion] quaternion]}
  ^Quaternion [^Quaternion p ^Quaternion q]
  (->Quaternion (c/+ ^double (:real p) ^double (:real q))
                (c/+ ^double (:imag p) ^double (:imag q))
                (c/+ ^double (:jmag p) ^double (:jmag q))
                (c/+ ^double (:kmag p) ^double (:kmag q))))

(defn plus
  "Add two quaternions"
  ^Quaternion [^Quaternion p ^Quaternion q]
  (Quaternion. (c/+ (.real p) (.real q))
               (c/+ (.imag p) (.imag q))
               (c/+ (.jmag p) (.jmag q))
               (c/+ (.kmag p) (.kmag q))))

(def q1 (->Quaternion 1 2 3 4))
(def q2 (->Quaternion 1 2 3 4))
(dotimes [_ 10] (time (dotimes [_ 1e7] (+ q1 q2))))
(println "------------------")
(dotimes [_ 10] (time (dotimes [_ 1e7] (plus q1 q2))))

"Elapsed time: 164.740521 msecs"
"Elapsed time: 110.912685 msecs"
"Elapsed time: 109.652137 msecs"
"Elapsed time: 109.653449 msecs"
"Elapsed time: 108.132582 msecs"
"Elapsed time: 108.504473 msecs"
"Elapsed time: 108.051357 msecs"
"Elapsed time: 494.658208 msecs"
"Elapsed time: 240.947768 msecs"
"Elapsed time: 108.906342 msecs"
------------------
"Elapsed time: 17.530754 msecs"
"Elapsed time: 12.345835 msecs"
"Elapsed time: 9.976968 msecs"
"Elapsed time: 9.843272 msecs"
"Elapsed time: 9.96787 msecs"
"Elapsed time: 9.890492 msecs"
"Elapsed time: 9.905521 msecs"
"Elapsed time: 9.949445 msecs"
"Elapsed time: 9.801012 msecs"
"Elapsed time: 10.023898 msecs"

[wedesoft]

Oh wow. I thought fixing all warnings was enough.

[bsless]

It only helps when you operate on primitives, if the argument is an Object then you'd get no warning.
Any keyword access returns a boxed object. (:real q) returns a Double, while (.real q) returns a double. Since it's stored as a double on the Quaternion, you box the value on any access, then unbox when you instantiate the quaternion or force it to be a primitive ^double.
Another way to look at it is why had ^double (:real p) cleaned the warnings? because the compiler saw an Object at the callsite.

[bsless]

Note I also overrode the constructor fn to not box the arguments

[wedesoft]

Ok, so far I thought accessing .real is slower than :real.

(def q (q/->Quaternion 1 2 3 4))
(time (dotimes [i 100000] (.real q)))
; "Elapsed time: 304.899835 msecs"
(time (dotimes [i 100000] (:real q)))
; "Elapsed time: 16.358152 msecs"

Although intuitively it would make more sense that the Java element accessor is better.

[bsless]

Your member access is reflective, try adding a type hint before q and you'll see the difference.
The compiler can track type hints in bindings, so it's sufficient to hint it in the function argument or in a let binding.

[wedesoft]

Ok, thanks for letting me know. Now it is faster.

(require '[sfsim.quaternion :as q])
(def q (q/->Quaternion 1 2 3 4))
(import '[sfsim.quaternion Quaternion])
(time (dotimes [i 1000000] (:real ^Quaternion q)))
; "Elapsed time: 21.568626 msecs"
(time (dotimes [i 1000000] (.real ^Quaternion q)))
; "Elapsed time: 13.780261 msecs"

[bsless]

The type hint for kw access doesn't do anything for the compile, it will compile to a keyword lookup site which ends up calling valAt and not the class field directly
https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/KeywordLookupSite.java
I also recommend wrapping a time/dotimes with another block of dotimes (for about 10 iters) to let the jit do its work.