ClojureScript

Collection hashers should use iterators

Details

  • Type: Enhancement Enhancement
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
  • Patch:
    Code

Description

In CLJS hash-ordered-coll and hash-unordered-coll currently consume their collections using seq instead of an iterator. Most core collection types have an efficient -iterator implementation which avoids the extra allocations inherent in seq.

Using iterators instead shows a modest performance improvement with larger collections and more frequent iterations; benchmark-runner size (1000) and repetition count (100) shows no perceptible difference on my machine (Ubuntu 18.04 using "Intel(R) Core(TM) i7-4710MQ CPU @ 2.50GHz")

cljs.benchmark-runner
(ns cljs.benchmark-runner
  (:refer-clojure :exclude [println]))

(def println print)

(set! *print-fn* js/print)


;; Note that coll size and repetition count are both
;; 10x what benchmark-runner usually uses. 
(def hash-coll-test
  (loop [i 0 r []]
    (if (< i 10000)
      (recur (inc i) (conj r (str "foo" i)))
      r)))
(def hash-imap-test
  (loop [i 0 r {}]
    (if (< i 10000)
      (recur (inc i) (conj r [(keyword (str "foo" i)) i]))
      r)))
(def hash-imap-int-test
  (loop [i 0 r {}]
    (if (< i 10000)
      (recur (inc i) (conj r [i i]))
      r)))
## benchmark run on Ubuntu 18.04 using "Intel(R) Core(TM) i7-4710MQ CPU @ 2.50GHz"
## benchmark results for master
Benchmarking with V8
[coll hash-coll-test], (hash-ordered-coll coll), 1000 runs, 1831 msecs
[coll hash-imap-test], (hash-unordered-coll coll), 1000 runs, 6506 msecs
[coll hash-imap-int-test], (hash-unordered-coll coll), 1000 runs, 8439 msecs

## benchmark results with this patch
Benchmarking with V8
[coll hash-coll-test], (hash-ordered-coll coll), 1000 runs, 1629 msecs
[coll hash-imap-test], (hash-unordered-coll coll), 1000 runs, 5753 msecs
[coll hash-imap-int-test], (hash-unordered-coll coll), 1000 runs, 6810 msecs

Activity

Hide
Mike Fikes added a comment -

cljs-2931.patch LGTM.

It passes in CI and Canary.

Here are the same three benchmarks, broadened to cover all of the VMs:

            V8: 1.18, 1.46, 1.23
  SpiderMonkey: 1.05, 0.94, 0.93
JavaScriptCore: 1.29, 1.03, 1.01
       Nashorn: 1.05, 1.31, 0.71
    ChakraCore: 1.26, 0.83, 0.80
       GraalVM: 1.05, 1.12, 1.18
Show
Mike Fikes added a comment - cljs-2931.patch LGTM. It passes in CI and Canary. Here are the same three benchmarks, broadened to cover all of the VMs:
            V8: 1.18, 1.46, 1.23
  SpiderMonkey: 1.05, 0.94, 0.93
JavaScriptCore: 1.29, 1.03, 1.01
       Nashorn: 1.05, 1.31, 0.71
    ChakraCore: 1.26, 0.83, 0.80
       GraalVM: 1.05, 1.12, 1.18
Hide
Mike Fikes added a comment -

cljs-2931.patch added to Patch Tender

Show
Mike Fikes added a comment - cljs-2931.patch added to Patch Tender

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated: