fuzz-aldrin-plus
fuzz-aldrin-plus copied to clipboard
Add support for Immutable.js
It would be really nice to seamlessly use this library with Immutable.js structures. Now we must call .toJS()
and that could be a bottleneck.
Ok what kind of structure do you have ? Array of string ? Array of object on which you query a key ?
I've never worked with immutable.js but we are 100% read only so I'm confident we can. Does it have a special way to detect collection is immutable and iterate on them ?
I'm thinking of providing a callback that could be used in a map like function.
var cb = fz.callBackFor(query,option) ImmutableCollectionOfCandidate.map( cb ).filter( .. ).sort( .. )
Altougth sorted probably need mutability
@jeancroy we have a list of maps (array of plain objects in pure JS). Immutable.js has sort method so that's not a problem. The only difference between array of plain objects and Immutable structures for this library are getters. For plain objects you use the dot sign object.name
for accessing properties but in Immutable you have to use .get('name')
.
Maybe we could pass some flag so you would know the structure is Immutable.
This is how I iterate over the list and extract the key to be ranked https://github.com/jeancroy/fuzzaldrin-plus/blob/master/src/filter.coffee#L16-L17
I could provide a "getterFunc" in the optionHash that input the element and output the value to be scored. Would that make sens, flexibility and performance wise ?
I will try to hack it and let you know if that's enough or something else needs to be changed.
Ok so here are results:
Changes in file fuzzaldrin.cofee:
Before:
filter: (candidates, query, options = {}) ->
return [] unless query?.length and candidates?.length
options = parseOptions(options)
filter(candidates, query, options)
After:
filter: (candidates, query, options = {}) ->
return immutable.List() unless query?.length and candidates?.size
options = parseOptions(options)
filter(candidates, query, options)
Changes in file filter.coffee are here https://www.diffchecker.com/899ruzir.
So now I'm not sure if we should try to put support for immutable.js into your lib or I should just fork it and call it something like immutable-fuzzaldrin. What do you think @jeancroy?
I'm looking at the Immutable documentation
If you need to apply a series of mutations locally before returning, Immutable gives you the ability to create a temporary mutable (transient) copy of a collection and apply a batch of mutations in a performant manner by using withMutations Important!: Only a select few methods can be used in withMutations including set, push and pop.
With this in mind i'd probably adapt the following line to use withMutations so you don't end up creating 300 different immutable list before sorting.
scoredCandidates = scoredCandidates.push({candidate, score})
Pushing on the idea that you want to avoid .toJS for performance reason I think we can do the following:
- First iterate on the Immutable collection (using forEach)
- Over each item execute an extract function
x=>x.get("key")
- Collect the score
- Push to a mutable array of <Immutable Map, Score>
- Map Pluck / take top N.
- Reseal the output "array of Immutable map" as "Immuatable collection of immutable map".
Basically let's assume you have 5000 items and at the end want to take the top 10. I'd much rather pay the price to re-seal the last 10 item then have immutable overhead on the temp array of 5000 items
Moreover this would increase the shared codepath between immutable / classic JS, so the immutable implementation could fit inside two function input transform (extract) and output transform ( array -> immutable collection)
The input /output transform could be re-used to support other data structures, maybe other implementation of immutable, or even complex / computed key object.x.y.z
What do you think ?
Basically what I'm trying to say is that by nature scoredCandidates is an object that will mutate once per item then .. as soon as it stop mutating... it get discarded with sort pluck and take.
so it's very anti immutable by nature, and not moving this one, I think it's possible to have easier cohabitation between normal, immutable and others.
@jeancroy Sounds like a better solution. I'm up for that 👍.
@jeancroy Any idea when this could be implemented?
Hi @petrbrzek just a head up.
There's now a es6 branch on which I redesigned the filtering.
There'll be support for cancelable async as well as iterating over array, anything that support es6 iterator and anything with .forEach method. They 'key' options also support functions, so one can write x=>x.prop
or x => x.get('prop')
I still have to write test & benchmark, but with those changes the library will officially support Immutable.js Sorry to be about 6 month late on the issue, I work on this as I have free time.
Also, Merry Christmas !