Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove stream laziness #469

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Remove stream laziness #469

wants to merge 1 commit into from

Conversation

lautis
Copy link
Member

@lautis lautis commented Nov 15, 2014

Based on discussed in #465, remove laziness from stream evaluation. The performance didn't change drastically. Sampling is slower, but simple things such as map and scan are faster. However, at least for map, similar performance boost could be achieved by removing instanceof checks from event classes.

After

diamond x 185 ops/sec ±2.51% (89 runs sampled)
combo x 211 ops/sec ±3.50% (87 runs sampled)
zip x 3,864 ops/sec ±1.28% (97 runs sampled)
flatMap x 315 ops/sec ±4.02% (80 runs sampled)
Bacon.combineTemplate.sample x 514 ops/sec ±0.84% (92 runs sampled)
Bacon.combineTemplate (deep) x 41.47 ops/sec ±0.65% (56 runs sampled)
Bacon.combineTemplate x 553 ops/sec ±1.06% (97 runs sampled)
EventStream.map x 8,997 ops/sec ±0.65% (96 runs sampled)
EventStream.scan x 7,703 ops/sec ±1.59% (94 runs sampled)
EventStream.toProperty x 7,569 ops/sec ±0.73% (96 runs sampled)

Before

diamond x 180 ops/sec ±3.87% (88 runs sampled)
combo x 211 ops/sec ±3.24% (86 runs sampled)
zip x 3,285 ops/sec ±2.94% (89 runs sampled)
flatMap x 300 ops/sec ±3.59% (76 runs sampled)
Bacon.combineTemplate.sample x 678 ops/sec ±1.58% (95 runs sampled)
Bacon.combineTemplate (deep) x 33.74 ops/sec ±2.42% (61 runs sampled)
Bacon.combineTemplate x 440 ops/sec ±2.46% (92 runs sampled)
EventStream.map x 7,196 ops/sec ±0.74% (89 runs sampled)
EventStream.scan x 5,861 ops/sec ±1.45% (95 runs sampled)
EventStream.toProperty x 6,201 ops/sec ±0.60% (98 runs sampled)

@lautis lautis force-pushed the eager branch 2 times, most recently from b8aa74e to ed78755 Compare November 15, 2014 11:37
@raimohanska
Copy link
Contributor

Did you try this with your work project? That's the performance that matters, after all.

@lautis
Copy link
Member Author

lautis commented Nov 15, 2014

Not yet. I could run some stress tests and see if this has any noticeable difference, but that would only show if there's any difference problems we've previously encountered. For a real test I'll have to wait to Monday evening when all the US people are online.

@lautis
Copy link
Member Author

lautis commented Nov 24, 2014

Performance seems better without laziness especially when the extra iteration in Bacon.when was removed. This isn't unexpected as Flowdock isn't really built to take advantage of it.

@AlexGalays
Copy link

Will this get merged in 0.8?

I guess it needs some update too, as things like this were added later: d49fd51

@lautis
Copy link
Member Author

lautis commented Dec 31, 2014

This is again mergeable: compiled JS had conflicts.

@lautis
Copy link
Member Author

lautis commented Feb 9, 2015

@raimohanska, any interest in merging this in? If so, I can rebase this to work on master.

@raimohanska
Copy link
Contributor

I think this might be one of the things to include into 0.8, as it's a quite a major change indeed. I suggest we create a version-0.8 branch for that stuff. Ok?

@lautis
Copy link
Member Author

lautis commented Feb 27, 2015

I've rebased this to work on master. This should also apply on the 0.8 branch (except for the generated JS).

@raimohanska
Copy link
Contributor

Thanks!

-juha-

On 28.2.2015, at 1.20, Ville Lautanala notifications@github.com wrote:

I've rebased this to work on master. This should also apply on the 0.8 branch (except for the generated JS).


Reply to this email directly or view it on GitHub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants