balancer / balancer-v3-monorepo

GNU General Public License v3.0
30 stars 7 forks source link

DRAFT: Optimize EnumerableMap to not load size #684

Closed elshan-eth closed 1 month ago

elshan-eth commented 1 month ago

Description

Maybe it is a strange idea, but what if we will store the length in the first element and pack it in one slit with an address?

Type of change

Checklist:

Issue Resolution

Closes #116

jubeira commented 1 month ago

This is pretty smart, thanks @elshan-eth.

I think the original point was to play around the idea that the maximum token amounts is 4, but if I'm not mistaken not knowing the actual length doesn't play along nicely with unchecked_At, and using the safe version would defeat the purpose?

I'm not against this one, but I'd like to import the original tests from OZ and test our modified implementation against it if possible.

elshan-eth commented 1 month ago

@jubeira, yes, we still need to know the length since we use it to fill arrays (PoolDataLib.load). We don't use safe operations because they are more expensive than unchecked_At.

elshan-eth commented 1 month ago

We have the same tests as openzeppelin tests in test/EnumerableMap.test.ts