ClojureScript

hash-set and PersistentHashSet/fromArray == faster set construction

Details

  • Type: Enhancement Enhancement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
  • Patch:
    Code and Test

Description

As discussed with mmarczyk in irc, the attached patch implements Clojure's hash-set function and modifies the compiler to create sets using a new fromArray static method. To test these, I created three new benchmarks. Here's before and after benchmark results:

$ time ./script/benchmark | grep '#{'
[], #{}, 100000 runs, 77 msecs
[], #{1 2 3}, 100000 runs, 582 msecs
[coll #{1 2 3}], (conj coll 4), 100000 runs, 104 msecs
[], #{}, 100000 runs, 740 msecs
[], #{1 2 3}, 100000 runs, 1809 msecs
[coll #{1 2 3}], (conj coll 4), 100000 runs, 221 msecs
[], #{}, 100000 runs, 281 msecs
[], #{1 2 3}, 100000 runs, 1310 msecs
[coll #{1 2 3}], (conj coll 4), 100000 runs, 288 msecs
./script/benchmark 109.41s user 3.82s system 127% cpu 1:28.96 total

$ time ./script/benchmark | grep '#{'
[], #{}, 100000 runs, 1 msecs
[], #{1 2 3}, 100000 runs, 333 msecs
[coll #{1 2 3}], (conj coll 4), 100000 runs, 100 msecs
[], #{}, 100000 runs, 1 msecs
[], #{1 2 3}, 100000 runs, 1670 msecs
[coll #{1 2 3}], (conj coll 4), 100000 runs, 325 msecs
[], #{}, 100000 runs, 1 msecs
[], #{1 2 3}, 100000 runs, 689 msecs
[coll #{1 2 3}], (conj coll 4), 100000 runs, 321 msecs
./script/benchmark 103.94s user 4.89s system 108% cpu 1:39.88 total
grep '#{' 0.00s user 0.00s system 0% cpu 1:39.85 total

Activity

Hide
Brandon Bloom added a comment -

Slightly better patch with the EMPTY case

Show
Brandon Bloom added a comment - Slightly better patch with the EMPTY case
Hide
David Nolen added a comment -

Getting a weird error when I try to run the tests. Also can we squash the commits? Thanks!

Show
David Nolen added a comment - Getting a weird error when I try to run the tests. Also can we squash the commits? Thanks!
Hide
Brandon Bloom added a comment -

Squashed patch.

Show
Brandon Bloom added a comment - Squashed patch.
Hide
Brandon Bloom added a comment -

Squashed and rebased on latest master. Is the rule no-multi-commit patches? Or simply just not for such small patches? In this case, I made and tested each each change in sequence. I also wanted the benchmarks to have a different sha1, so that the perf improvement would show up in the graphs. I'll try to provide squashed patches in the future.

What weird error are you getting? Works for me :-P

Show
Brandon Bloom added a comment - Squashed and rebased on latest master. Is the rule no-multi-commit patches? Or simply just not for such small patches? In this case, I made and tested each each change in sequence. I also wanted the benchmarks to have a different sha1, so that the perf improvement would show up in the graphs. I'll try to provide squashed patches in the future. What weird error are you getting? Works for me :-P
Hide
Brandon Bloom added a comment -

Updating 2nd benchmark in description to include the EMPTY optimization

Show
Brandon Bloom added a comment - Updating 2nd benchmark in description to include the EMPTY optimization

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: