Simpler and faster filtering, mapping, skipping and limiting

[jacoscaz]

While researching #44 , @jeswr discovered that the performance of .map() and .filter() could be greatly improved by adding simple dedicated iterators for mapping and filtering, respectively. The impact of using these simplified iterators grows with the length of the chain of iterators.

As measured on a 13'' 2020 MacBook Pro (Apple Silicon, M1, 16 GB RAM), this PR makes the following test run in ~30ms. Before this PR the same test would terminate after ~300ms - a 10x difference.

let i = 0;
const arr = new Array(200000).fill(true).map(() => i++);

class RangeIterator<O> extends AsyncIterator<O> {
  constructor(source: O[]) {
    super();
    let i = 0;
    let l = source.length;
    this.read = (): O | null => {
      if (i >= l) {
        this.close();
        return null;
      }
      return source[i++];
    };
    Promise.resolve().then(() => {
      this.emit('readable');
    });
  }
}

const iterator = new RangeIterator(arr)
  .map((item: number) => item)
  .map((item: number) => item)
  .map((item: number) => item)
  .map((item: number) => item)
  .map((item: number) => item)
  .map((item: number) => item)
  .map((item: number) => item)
  .map((item: number) => item)
  .map((item: number) => item)
  .map((item: number) => item);

const now = Date.now();
iterator.on('data', () => {}).on('end', () => {
  console.log('elapsed', Date.now() - now);
});

Note that the RangeIterator class is just a stopgap for #47 .

[jacoscaz]

@RubenVerborgh if you like where this is going I can spend some time refining those new iterators (cleaning up event handlers, implementing read() as an actual class method, ...).

[RubenVerborgh]

if you like where this is going I can spend some time refining those new iterators (cleaning up event handlers, implementing read() as an actual class method, ...).

@jacoscaz Yes please. However, is this still relevant after #50?

[jacoscaz]

#50 is still about TransformIterator, which extends BufferedIterator. The gains in #46 will make the impact of this PR less significant for sure but there should still be a difference between buffering and no buffering. I'll do some comparison between the state of things now that #46 has been merged and this PR (and also #49 ) and report back.

[coveralls]

Coverage remained the same at 100.0% when pulling d7ab7d9 on jacoscaz:faster-filtering-mapping into 8b01b5e on RubenVerborgh:main.

[jacoscaz]

With main as it is right now, after merging #46 , going through the following chain takes ~210 ms with an array of 200k items (MacBook Pro, Apple Silicon, M1, 16 GB RAM). With this PR that goes down to ~30 ms, so roughly 7x faster.

new ArrayIterator(arr)
      .map((item: number) => item)
      .map((item: number) => item)
      .map((item: number) => item)
      .map((item: number) => item)
      .map((item: number) => item)
      .map((item: number) => item)
      .map((item: number) => item)
      .map((item: number) => item)
      .map((item: number) => item)
      .map((item: number) => item)

[jacoscaz]

@RubenVerborgh this should be in a better place now. Those new iterators likely deserve dedicated tests but before that, should anything more be done In _destroy() when it comes to event handling?

[jacoscaz]

This PR now includes @jeswr work from #53 , focused on combining subsequent map() and filter() operations into one single iterator.

[jacoscaz]

Superseded by #57