ClojureScript

Transient support for PersistentHashMap

Details

  • Type: Enhancement Enhancement
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code

Description

Follow-up to CLJS-178 (the ClojureScript port of Clojure's PersistentHashMap) introducing transient support for PersistentHashMap.

The attached patch applies cleanly on top of the latest transient-less patch from CLJS-178. The code is also viewable on GitHub – here's the current branch:

https://github.com/michalmarczyk/clojurescript/tree/phm2-transient

And here's a jsPerf test comparing PHM vs. THM on the task of building a map of 10000 entries (with THM being twice as fast in Chrome):

http://jsperf.com/cljs-persistent-hash-map-transient-support

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
David Nolen added a comment -

Nice, could we add some tests?

Show
David Nolen added a comment - Nice, could we add some tests?
Hide
Permalink
Michał Marczyk added a comment -

Sure, I'll get onto it tonight.

Show
Michał Marczyk added a comment - Sure, I'll get onto it tonight.
Hide
Permalink
Michał Marczyk added a comment -

Same code as the previous patch but applies on top of the current master.

Here's a branch for convenience:

https://github.com/michalmarczyk/clojurescript/tree/phm-transient-integration

Will do the tests now.

Show
Michał Marczyk added a comment - Same code as the previous patch but applies on top of the current master. Here's a branch for convenience: https://github.com/michalmarczyk/clojurescript/tree/phm-transient-integration Will do the tests now.
Hide
Permalink
Michał Marczyk added a comment -

The new 0001-... patch fixes a couple of typos which prevented {dissoc!} and {get} from working with TransientHashMap.

The 0002-... patch introduces a suite of tests for PHM & THM. Happily the PHM passes them all without modification.

Show
Michał Marczyk added a comment - The new 0001-... patch fixes a couple of typos which prevented {dissoc!} and {get} from working with TransientHashMap. The 0002-... patch introduces a suite of tests for PHM & THM. Happily the PHM passes them all without modification.
Hide
Permalink
Michał Marczyk added a comment -

Oh, one more thing – here's the new branch:

https://github.com/michalmarczyk/clojurescript/tree/phm-transient-integration-2

Show
Michał Marczyk added a comment - Oh, one more thing – here's the new branch: https://github.com/michalmarczyk/clojurescript/tree/phm-transient-integration-2
Hide
Permalink
Michał Marczyk added a comment -

A cleaner patch w/ tests w/o an unnecessary assertion msg. Branch @

https://github.com/michalmarczyk/clojurescript/tree/phm-transient-integration-3

Show
Michał Marczyk added a comment - A cleaner patch w/ tests w/o an unnecessary assertion msg. Branch @ https://github.com/michalmarczyk/clojurescript/tree/phm-transient-integration-3
Hide
Permalink
Michał Marczyk added a comment -

Hm, actually I missed one thing in the tests – hash collisions. Will prepare a new patch in a moment.

Show
Michał Marczyk added a comment - Hm, actually I missed one thing in the tests – hash collisions. Will prepare a new patch in a moment.
Hide
Permalink
Michał Marczyk added a comment -

Just a heads-up – apparently there's something the matter with hash collision handling in transients. Will attach a new patch once I get this sorted.

Show
Michał Marczyk added a comment - Just a heads-up – apparently there's something the matter with hash collision handling in transients. Will attach a new patch once I get this sorted.
Hide
Permalink
David Nolen added a comment - - edited

Moving forward could we replace the patches instead of always adding new ones? It makes it very hard to figure out which one to use especially on tickets that have a long life like the original PHM one.

Show
David Nolen added a comment - - edited Moving forward could we replace the patches instead of always adding new ones? It makes it very hard to figure out which one to use especially on tickets that have a long life like the original PHM one.
Hide
Permalink
Michał Marczyk added a comment -

Oh. Sure thing. I'll replace the whole lot once I've got the new patch ready then.

Show
Michał Marczyk added a comment - Oh. Sure thing. I'll replace the whole lot once I've got the new patch ready then.
Hide
Permalink
Michał Marczyk added a comment -

These patches apply cleanly against the current master. The current branch is @

https://github.com/michalmarczyk/clojurescript/tree/phm-transient-integration-5

Show
Michał Marczyk added a comment - These patches apply cleanly against the current master. The current branch is @ https://github.com/michalmarczyk/clojurescript/tree/phm-transient-integration-5

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4#649-r158309) | Report a problem
Powered by a free Atlassian JIRA open source license for Clojure. Try JIRA - bug tracking software for your team.