Clojure

Use transients in merge and merge-with

Details

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

Description

It would be nice if merge used transients.

Activity

Hide
Jason Wolfe added a comment -

I will take a crack at a patch today.

Show
Jason Wolfe added a comment - I will take a crack at a patch today.
Hide
Jason Wolfe added a comment - - edited

This patch (transient-merge.diff) makes merge, merge-with, and zipmap (since it was right there and could obviously benefit from transients as well) use transients.

Three potential issues:

  • I had to move the functions, since they depend on transient and friends. I assume this is preferable to a forward declaration. This was the best place I could find, but happy to move them elsewhere.
  • I added multiple arities, to avoid potential performance cost of transient-ing a single argument. Happy to undo this if desired.
  • I had to slightly alter the logic in merge-with, since transient maps don't support contains? (or find).
Show
Jason Wolfe added a comment - - edited This patch (transient-merge.diff) makes merge, merge-with, and zipmap (since it was right there and could obviously benefit from transients as well) use transients. Three potential issues:
  • I had to move the functions, since they depend on transient and friends. I assume this is preferable to a forward declaration. This was the best place I could find, but happy to move them elsewhere.
  • I added multiple arities, to avoid potential performance cost of transient-ing a single argument. Happy to undo this if desired.
  • I had to slightly alter the logic in merge-with, since transient maps don't support contains? (or find).
Hide
Michał Marczyk added a comment - - edited

I posted a separate ticket for zipmap, with patch, on 30/May/12: CLJ-1005.

Show
Michał Marczyk added a comment - - edited I posted a separate ticket for zipmap, with patch, on 30/May/12: CLJ-1005.
Hide
Jason Wolfe added a comment -

Ah, sorry if I overstepped then. Happy to remove that change from this patch then if that will simplify things – just let me know.

Show
Jason Wolfe added a comment - Ah, sorry if I overstepped then. Happy to remove that change from this patch then if that will simplify things – just let me know.

People

Vote (0)
Watch (3)

Dates

  • Created:
    Updated: