clojure-dev

Issues: https://clojure.atlassian.net/browse/CLJ | Guide: https://insideclojure.org/2015/05/01/contributing-clojure/
borkdude 2019-02-25T21:43:53.057600Z

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))))

borkdude 2019-02-25T21:45:08.058200Z

1.10.0:

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

borkdude 2019-02-25T21:47:07.058500Z

seems like roughly a 5% speedup

borkdude 2019-02-25T21:52:09.059200Z

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

borkdude 2019-02-25T21:53:56.059600Z

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

alexmiller 2019-02-25T22:02:25.060800Z

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.