Fork me on GitHub
#clojure-dev
<
2019-09-23
>
ghadi14:09:55

there are a couple of avenues of things that are worth looking into for merge: transients && IReduceKV based impls

ghadi14:09:47

Scala's new PHMs do merging based on the actual tree nodes themselves. Might be worth a look, too. Michael Steindorfer is the reference for those papers

ikitommi15:09:16

thanks for the pointer, will check those out (out of common interest)

ghadi14:09:26

changing the impl of Map is a non-starter, imho

ghadi14:09:18

there's some aborted work in a ticket on transients + map merge, but it needs more analysis

ghadi14:09:10

one bounding constraint is that the conj operation (merge {:a :b} [:c :d]) remains valid

borkdude13:09:14

why should that a bounding constraint? I've only seen this conj-y way of using merge in beginner's 4clojure solutions. I came across those when validating specs in https://github.com/borkdude/speculative. I've haven't come across this usage anywhere else.

borkdude13:09:34

The docstring says merge is intended for maps.

borkdude13:09:35

I know docstrings don't always paint the complete picture, but combined evidence ...

ghadi13:09:56

compatibility

borkdude13:09:13

some maybe it's better to introduce a merge2 then for the hypothetical case anyone was using (merge {} [:a 1])?

ghadi13:09:21

it's not a hypothetical case, I've come across many usages in the wild

ghadi13:09:47

whether they were conscious about it, I can't say, but we can't break code

ikitommi15:09:14

;; 8ns
(cc/quick-bench
  (reduce-kv assoc {} {}))

ghadi15:09:23

The best immediate way to contribute to that ticket is a benchmark suite featuring arguments of different sizes & types to find the baselines.

👌 8