ClojureScript

Update merge-with to use key / val

Details

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

Description

Update merge-with's implementation to use key / val instead of first / second.

Rationale: Expected perf benefit. Secondarily, closer fidelity with Clojure. This is likely an OK change now that the shipping map implementations produce map entries.

Activity

Hide
Mike Fikes added a comment -

Speedup with patch for

(simple-benchmark [m1 {:a 2, :b 3, :c 5}
                   m2 {:a 7, :b 11, :d 13, :e 17}
                   m3 {:a 19, :d 23, :f 29}]
  (merge-with + m1 m2 m3)
  1e5)

is

            V8: 1.36
  SpiderMonkey: 1.40
JavaScriptCore: 1.34
       Nashorn: 1.27
    ChakraCore: 1.39
       GraalVM: 1.11
Show
Mike Fikes added a comment - Speedup with patch for
(simple-benchmark [m1 {:a 2, :b 3, :c 5}
                   m2 {:a 7, :b 11, :d 13, :e 17}
                   m3 {:a 19, :d 23, :f 29}]
  (merge-with + m1 m2 m3)
  1e5)
is
            V8: 1.36
  SpiderMonkey: 1.40
JavaScriptCore: 1.34
       Nashorn: 1.27
    ChakraCore: 1.39
       GraalVM: 1.11
Hide
Mike Fikes added a comment - - edited

Example code that relies on the fact that merge-with is internally using first / second which would break is

(->> [:a 1 :b 2] (partition 2) (merge-with vector {}))

Code like this currently in the wild: https://github.com/gf3/secretary/blob/1f2036a694e49f58a97c9401878602148f9d6310/src/secretary/core.cljs#L252

Downstream ticket for the above: https://github.com/gf3/secretary/issues/100

Show
Mike Fikes added a comment - - edited Example code that relies on the fact that merge-with is internally using first / second which would break is
(->> [:a 1 :b 2] (partition 2) (merge-with vector {}))
Code like this currently in the wild: https://github.com/gf3/secretary/blob/1f2036a694e49f58a97c9401878602148f9d6310/src/secretary/core.cljs#L252 Downstream ticket for the above: https://github.com/gf3/secretary/issues/100
Hide
Mike Fikes added a comment -

Passes CI an Canary tests.

Show
Mike Fikes added a comment - Passes CI an Canary tests.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: