perf: Add a map coins struct to speedup bank genesis

[ValarDragon]

Description

This PR introduces a map coins struct, and uses it in the bank keepers genesis.

The point of this PR is to significantly speedup bulk add operations. Today every add requires a full heap copy of the operands being added together. (And forces addition to therefore be linear in both sizes)

So if were doing N additions, each containing O(1) independent coins, this takes O(N^2) time right now. This is quite expensive for some operations, such as InitGenesis. It causes about 1% of the time delay of an Osmosis InitGenesis (which is quite expensive)

This adds up quite fast! After this PR, the time-delay for AddCoins in bank genesis is negligible. (InitBalances is still time delay, understandably. However that was always less than the contribution than these AddCoins)

---

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

• included the correct type prefix in the PR title
• added ! to the type prefix if API or client breaking change
• targeted the correct branch (see PR Targeting)
• provided a link to the relevant issue or specification
• followed the guidelines for building modules
• included the necessary unit and integration tests
• added a changelog entry to CHANGELOG.md
• included comments for documenting Go code
• updated the relevant documentation or specification
• reviewed "Files changed" and left comments if necessary
• confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

• confirmed the correct type prefix in the PR title
• confirmed ! in the type prefix if API or client breaking change
• confirmed all author checklist items have been addressed
• reviewed state machine logic
• reviewed API design and naming
• reviewed documentation is accurate
• reviewed tests and test coverage
• manually tested (if applicable)

[yihuang]

How about store the coins in a slice first, then add them in one call? We can use sth like hash map as an implementation detail of Add?

[08d2]

Can you add a test or benchmark that validates your claims and prevents regressions?

[ValarDragon]

@08d2 tests are added for MapCoins, in particular every AddCoins test is applied there. Also covered by the Bank InitGenesis tests.

Benchmarked in larger context, but I can add one here sure. (The asymptotics are clear though)

@yihuang I'm not sure how to do that without making sdk.Coins an enum type over Map / []sdk.Coin. Unless you mean making a bulk add operation, but that would require more heap allocations in the bank InitGenesis context!

[ValarDragon]

Its a pretty significant speedup:

BenchmarkSumOfCoinAdds/Fn:_MapCoins,_num_adds:_1000,_coinsPerAdd:_5,_intersecting:_2-16         	     829	   1304745 ns/op	  484129 B/op	    4100 allocs/op
BenchmarkSumOfCoinAdds/Fn:_Coins,_num_adds:_1000,_coinsPerAdd:_5,_intersecting:_2-16            	       2	 507666448 ns/op	379050596 B/op	 6024868 allocs/op
BenchmarkSumOfCoinAdds/Fn:_MapCoins,_num_adds:_10000,_coinsPerAdd:_10,_intersecting:_10-16      	     134	   9001058 ns/op	 8001232 B/op	  199984 allocs/op
BenchmarkSumOfCoinAdds/Fn:_Coins,_num_adds:_10000,_coinsPerAdd:_10,_intersecting:_10-16         	      27	  43774524 ns/op	44155787 B/op	  769925 allocs/op

I tried making benchmarks with more non-intersecting coins, and they wouldn't finish in reasonable timeframes with sdk.Coins due to the N^2 behavior. (Worked fine with mapcoins)

The main remaining mapcoins bottleneck is heap allocations for int adds, which could also be removed, once there is a mutative int add operation

[yihuang]

@yihuang I'm not sure how to do that without making sdk.Coins an enum type over Map / []sdk.Coin. Unless you mean making a bulk add operation, but that would require more heap allocations in the bank InitGenesis context!

I see, makes sense.

[alexanderbez]

once there is a mutative int add operation

Would you be opposed to adding this one API in this PR since it's a very hot path?

[ValarDragon]

I'm happy to add that in! Alright if I do so in a separate PR though? Currently doing other things to speedup Genesis work / would prefer this get through on its own :)

[alexanderbez]

I'm happy to add that in! Alright if I do so in a separate PR though? Currently doing other things to speedup Genesis work / would prefer this get through on its own :)

Sure thing 👍

[mark-rushakoff]

There are a couple small cleanup opportunities here, but no big changes needed.

[mark-rushakoff]

While this is clearly backed by a map, the name MapCoins doesn't indicate the intent of the type. Maybe BulkCoins or BulkAddCoins would be clearer.

[ValarDragon]

Sure, happy to switch to anything

[tac0turtle]

LGTM

[julienrbrt]

@ValarDragon can you run make lint-fix?