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

encodeFunctionData v5 vs v4 performance #1012

Closed
johngrantuk opened this issue Aug 20, 2020 · 5 comments
Closed

encodeFunctionData v5 vs v4 performance #1012

johngrantuk opened this issue Aug 20, 2020 · 5 comments
Labels
enhancement New feature or improvement. fixed/complete This Bug is fixed or Enhancement is complete and published.

Comments

@johngrantuk
Copy link

Thanks for all your work on this library.
We’ve been using ethers v4 with MakerDao multicall to aggregate a large amount of calls at once (currently around 4324 calls).
When migrating to ethers v5 I seem to get a larger delay and a large spike in CPU usage when I call the aggregate function using Contract.aggregate().
Digging into it further it looks like the intensive part is at populateTransaction in @ethersproject/contracts where it calls encodeFunctionData.
I wrote a couple of tests to compare a function encode for v4 and v5 and it does appear to be quite a difference and I wondered if there was anything you could recommend to overcome this or maybe I’m doing something wrong?

Test examples can be easily run using code from this repo: https://github.com/balancer-labs/ethersjs-test

Example output:
V5:
CPU usage encodeFunctionData: { user: 7027636, system: 1674509 }
encode: 4570.885ms
V4:
CPU usage encodeFunctionData: { user: 842372, system: 31249 }
encode: 510.924ms

Any advice would be much appreciated. Thanks!

@ricmoo
Copy link
Member

ricmoo commented Aug 20, 2020

Someone from state channels asked something similar recently.

There is no change I can think of that should have caused this, but can investigate.

Also, if you want to investigate too and notice any part in particular that seems to be more expensive, please let me know. :)

@johngrantuk
Copy link
Author

Thanks for that!

I did try to dig further and it looked like the coder.encode function maybe be the most expensive -
https://github.com/ethers-io/ethers.js/blob/ethers-v5-beta/packages/abi/src.ts/abi-coder.ts#L106
which in this case is a tuple encoder. I got a bit lost comparing the encode/pack function to the v4 version to see the difference but I'll keep trying. Let me know if there's anything specifically useful I could help with.

@ricmoo ricmoo added investigate Under investigation and may be a bug. enhancement New feature or improvement. labels Aug 20, 2020
@johngrantuk
Copy link
Author

Been digging a bit further. It seems like _writeData in abstract-coder.ts is quite costly, I’m guessing because of the concat function, especially when its a large amount of data. Replacing it with something like below seems to give a decent improvement:

_writeData(data: Uint8Array): number {
let newLen = this._data.length + data.length;
let old = this._data;
this._data = new Uint8Array(newLen);
this._data.set(old, 0);
this._data.set(data, old.length);
return data.length;
}

Let me know if this looks like a reasonable approach or you have any thoughts? Thanks

@ricmoo ricmoo added the on-deck This Enhancement or Bug is currently being worked on. label Oct 3, 2020
@ricmoo ricmoo removed the investigate Under investigation and may be a bug. label Oct 5, 2020
@ricmoo
Copy link
Member

ricmoo commented Oct 6, 2020

The performance should be much better now in 5.0.16. Try it out and let me know! :)

@ricmoo ricmoo added fixed/complete This Bug is fixed or Enhancement is complete and published. and removed on-deck This Enhancement or Bug is currently being worked on. labels Oct 6, 2020
@johngrantuk
Copy link
Author

Looks great! Many thanks for all your help around this :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement. fixed/complete This Bug is fixed or Enhancement is complete and published.
Projects
None yet
Development

No branches or pull requests

2 participants