updated process_verified_transactions to utilize existential type

[rlkelly]

since this was collected and immediately mapped, it makes sense to preserve the iterator until you are ready to fully collect

[garious]

Fancy. :) I'll give some thought to how much of this we want. It makes the code a bit intimidating, so it'd be good to pair a change like this with a benchmark that showcases its value. Thanks for getting the ball rolling!

[rlkelly]

I was kind of curious about that myself, I'll see what kind of improvement this makes

[rlkelly]

test bank::bench::process_verified_event_new_bench        ... bench:   5,970,687 ns/iter (+/- 247,295)
test bank::bench::process_verified_event_old_bench        ... bench:   6,087,549 ns/iter (+/- 485,155)
test bank::bench::process_verified_transactions_new_bench ... bench:   6,797,794 ns/iter (+/- 381,167)
test bank::bench::process_verified_transactions_old_bench ... bench:   6,781,942 ns/iter (+/- 365,668)

so not much performance gain :-/

[rlkelly]

I cranked up the number of iterations significantly to see how it scaled, seems to be linear:

test bank::bench::process_verified_event_new_bench        ... bench:   6,548,177 ns/iter (+/- 359,798)
test bank::bench::process_verified_event_old_bench        ... bench:   6,617,120 ns/iter (+/- 282,318)
test bank::bench::process_verified_transactions_new_bench ... bench: 682,921,744 ns/iter (+/- 61,173,740)
test bank::bench::process_verified_transactions_old_bench ... bench: 683,754,552 ns/iter (+/- 168,575,016)

[garious]

Thanks for doing this analysis! My thinking is that we shouldn't move forward with this one, but it's a great technique we can keep in our back pocket. One concern I have here is that we'd do all this work for marginal gains and in the meantime, if the Rust team goes and implements the following optimization, we'd want to go put all the code back the way it was.

http://citeseer.ist.psu.edu/viewdoc/summary?doi=10.1.1.104.7401

[rlkelly]

So for posterities sake I made a longer example just to see if anything changed.

test bank::bench::process_verified_transactions_new_bench ... bench: 7,391,736,717 ns/iter (+/- 443,474,208)
test bank::bench::process_verified_transactions_old_bench ... bench: 7,966,856,763 ns/iter (+/- 654,743,947)

I'm assuming that's a Haskell optimization that hasn't yet been applied to Rust?

[garious]

I don't think this benchmark is letting you see the value of getting rid of that malloc. To verify the Haskell optimization doesn't yet exist in Rust, you'd compare:

a.into_iter().map(|x| x + 1).collect().into_iter().map(|x| x * 2).collect()

To:

a.into_iter().map(|x| x + 1).map(|x| x * 2).collect()

Add to that the map f . map g == map (f . g) optimization that's also easy in Haskell:

a.into_iter().map(|x| (x + 1) * 2).collect()

The compiler has enough information to do both optimizations. I'm guessing that it currently does neither. If it already did deforestation then they'd probably apply their zero-cost abstraction philosophy to enable the syntax:

a.map(|x| x + 1).map(|x| x * 2)

To reiterate my previously-mentioned concern, if Rust were to implement both deforestation and map f . map g == map (f . g), then using existentials would just be the verbose syntax of the same thing. So to move forward in this direction is to say both that we thing existentials is a big win and that we don't think the Rust team will implement these optimizations in the near future.