Feat transform iterator #50
Conversation
|
I tried to add some features for throwing extra errors/warnings in production mode in 3a57d17 - but I couldn't quite figure out how to configure the configs and test-suite properly to handle this. |
|
@jeswr Thanks! Do we have any insights into the performance of this one? |
There was a problem hiding this comment.
So I like the idea, but I think mutation unnecessarily complicates things.
Suggestion: can we instead override map and filter methods on the original TransformIterator, with a check that if the source already is instanceof TransformIterator that we create this more optimized iterator instead?
For instance, source.map(x => x * 2).map(y => y + 1). Then the second map recognizes that the input is already a TransformIterator, so what it actually returns is a TransformIterator over source (!) with as map function the combination of the two mappings. So basically source.map(combine(x => x * 2, y => y + 1)).
| } | ||
|
|
||
| read(): T | null { | ||
| const func = build(this.transforms); |
There was a problem hiding this comment.
We can cache this instead.
| Transforms items according to a synchronous generator (hence no need for buffering) | ||
| @param {Function} transform The function to transform items with | ||
| */ | ||
| syncTransform<D>(transform: (item: T) => Generator<D>): FastTransformIterator<D> { |
There was a problem hiding this comment.
What are the use cases for this transformation? Is this just a multimap?
On the experiments I did I saw up to a 2x improvement in some of the tests I did a few days ago when compared to the chained use of MapIterator and FilterIterator (I haven't done benchmarks for the transform). I'd imagine that the performance increase is somewhat proportional to the number of elements you are filtering out as well since this essentially eliminates any upstream |
Per discussion in #44 - a fast transform iterator that uses mutations. Happy to take suggestions for renaming.
@RubenVerborgh This requires modifying some of the package.json and tsconfig.json settings. So it may be better to do this in a separate PR.
This code could probably do with some more integration tests in the future - but not sure if it is worth bothering until the changes in #45 get integrated.
Pinging @jacoscaz