Map transducing

[benlesh]

As a PoC, I've implemented transducing on map!

In this PR:

1. Chained map operators will collapse on themselves. This results in about a 20% performance boost when only two map operations are chained. That should grow with the number chained.
2. map().mergeAll() will flatten to .mergeMap() via magic. This results in about a 20% perf improvement over using .map().mergeAll() prior to this commit, and is ~15x faster than .map().mergeAll() in Rx4.

While I think we can land this here in master, this is a PoC to see what needs to be done.

One thing I learned while working on this is the thisArg makes creating this a little uncomfortable. As if I needed another reason to hate the thisArgs.

[benlesh]

attn @trxcllnt (for sanity).

attn @benjamingr, @robwormald and @jeffbcross as this relates to discussion in https://github.com/angular/http/issues/56.

[benjamingr]

Awesome! Thanks so much for exploring this, %20 performance boost for map operators collapsing sounds like a useful optimization. What about filter and its friends?

Optimally this optimization for single subscriber is something that should be done more generally.

[benlesh]

@benjamingr if this PR passes the smell test then scan, reduce, filter can get the same treatment and: map().concat() can be concatMap, map().switch() can be switchMap(), etc scan().mergeAll() can be mergeScan().

Another possibility is collapsing resultSelectors from operators like zip or combineLatest with map... (e.g. .zip(a, b, (a, b) => a + b).map(x => x + '!') is really just .zip(a, b, (a, b) => (x => x + '!')(a + b)))

[benlesh]

In the case of zip above, it will take a different type of approach, because we can't skip to the source operator, rather we're augmenting output.

[luisgabriel]

@Blesh Did you see if there is any significant change in performance when map is not chained (e.g. only a single map)?

[benlesh]

@luisgabriel I did not... but the map perf test was already chaining them. Given that it's really just a quick assertion at the moment map() is called, I doubt we'd see any noticeable difference in any perf test. Especially, since we're generally not calling map in the perf tests, rather we're testing the usage of the Observable result of the map call.

[benjamingr]

@luisgabriel looking at the source code change - when no chain collapsing is performed the same path is hit - it should perform exactly the same. I'm not sure if these two extra variables are needed but realistically the runtime can take care of that for us.

[trxcllnt]

@Blesh is there a method we can add to the Operator interface to support this more generically (and move the conditional transducing logic into the Operator implementation instead of the public method)?

[benlesh]

@trxcllnt definitely. Perhaps call could just return a different Subscriber?

[trxcllnt]

@Blesh It could, but don't the operators need to know about each other? I'm thinking something like:

export function map(project) {
  return this.lift(this.operator.transduce(new MapOperator(project)));
}

And the Operator base class could have a no-op transduce implementation:

class Operator {
  call(subscriber) { return subscriber; }
  transduce(operator) {
    return operator; // return the operator we were given if we can't collapse it with ourself.
  }
}

[trxcllnt]

@Blesh though I'm not sure about how much I like the word transduce for the public API. I know it's well understood in certain circles, but another term might make more sense for newcomers trying to implement their own operators... collapse perhaps?

[benjamingr]

+1 for collapse but for the opposite reason, transduce is well understood enough to trigger bikeshed later on.

[luisgabriel]

@Blesh @benjamingr Hmm. Makes sense.

[benlesh]

Looking at this design, I think it falls apart a bit with operators like filter and scan. I'll leave this open, but I don't think it'll make it into this release.

[benjamingr]

Ping @Blesh status?

[benlesh]

@benjamingr I've been working on other things, but there's a new design in the works for this that (hopefully) will be able to collapse almost all sync map-type operations in a chain.

[masaeedu]

@Blesh Is there an issue where you're discussing/tracking this?

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.