Fork me on GitHub
#clojure-dev
<
2019-02-25
>
borkdude21:02:53

I’ve been trying an override in PersistentHashMap of cons:

public IPersistentCollection cons(Object o){
        if(o instanceof Map.Entry)
            {
		Map.Entry e = (Map.Entry) o;

		return assoc(e.getKey(), e.getValue());
            }
        TransientHashMap t = new TransientHashMap(this);
        for(ISeq es = RT.seq(o); es != null; es = es.next())
            {
		Map.Entry e = (Map.Entry) es.first();
		t.doAssoc(e.getKey(), e.getValue());
            }
	return t.doPersistent();
    }
Code I tested it with:
(let [m1 (zipmap (range 100) (range 100)) m2 (zipmap (range 200) (range 200))] (time (dotimes [_ 100000] (merge m1 m2 m1 m2))))

borkdude21:02:08

1.10.0:

"Elapsed time: 4246.61557 msecs"
1.11.0-master-SNAPSHOT with this patch:
"Elapsed time: 3984.354273 msecs"

borkdude21:02:07

seems like roughly a 5% speedup

borkdude21:02:09

With criterium:

user=> (let [m1 (zipmap (range 100) (range 100)) m2 (zipmap (range 200) (range 200))] (quick-bench (merge m1 m2 m1 m2)))
;; 1.10.0
Evaluation count : 14172 in 6 samples of 2362 calls.
             Execution time mean : 44.637929 µs
    Execution time std-deviation : 2.639729 µs
   Execution time lower quantile : 42.215871 µs ( 2.5%)
   Execution time upper quantile : 47.827785 µs (97.5%)
                   Overhead used : 1.716963 ns
;; vs patch:
Evaluation count : 15390 in 6 samples of 2565 calls.
             Execution time mean : 39.645241 µs
    Execution time std-deviation : 517.214458 ns
   Execution time lower quantile : 39.034372 µs ( 2.5%)
   Execution time upper quantile : 40.262884 µs (97.5%)
                   Overhead used : 1.677237 ns

borkdude21:02:56

Looks like a 10% improvement, maybe worth a JIRA ticket or not?

Alex Miller (Clojure team)22:02:25

sure. btw, merging m1 m2 m1 m2 is not any more elements than just m1 m2. would probably be more interesting to have those be better.