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

chore: reduce time spent in synchronous code #2627

Closed
MartinquaXD opened this issue Apr 17, 2024 · 1 comment
Closed

chore: reduce time spent in synchronous code #2627

MartinquaXD opened this issue Apr 17, 2024 · 1 comment

Comments

@MartinquaXD
Copy link
Contributor

Background

Since we are routing almost all the quotes through the driver we are seeing a few performance issues.
Currently my best guess is that we are spending a bunch of time in synchronous code which can clog the async runtime somewhat.
I built a very crude test setup which has all the major liquidity sources enabled and a colocated baseline solver as a quoter. Then I built a script that sends USDC -> USDT quote requests to the API with random sell amounts (to avoid caching).
I ran the script for 5 minutes and generated flamegraphs to show which optimizations could the most impactful.

I identified 3 major issues:

Creating contract instances using ethcontract-rs

To call contract methods we currently have to instantiate contract instances. This is done with Contract::at(web3, address) and can be extremely costly. The reason is that although there is 0 dynamic data associate with correctly calling a contract method ethcontract-rs relies on runtime values a lot. And creating those runtime values (contract instances) is very costly for contracts with many methods.

Ideally we would replace ethcontract-rs with something like alloy for calling contract methods in a type safe manor because alloy does not involve any runtime logic.

Converting from old types to new types

In the driver we currently still have the boundary layer which converts legacy cold to types which fit nicely into our new domain layer. This conversion is pure overhead since eventually the old legacy code should not exist anymore. My tests showed that converting the uniswapv3 liquidity is the most costly so that is the one we should convert first.

Serializing requests

Currently we send liquidity to solvers which rely on that. For auctions this is not a huge deal since auctions happen relatively rarely but quotes can happen 20 times per second so the serialization overhead hurts way more for those requests.
The most basic solution would probably be to introduce a new serialization format which is more performant but this obviously a huge breaking change so it would have to be opt-in but at least for the baseline solver it could be a huge gain.
This change obviously needs to be discussed extensively but could result in very significant performance improvements.

flamegraph_original

MartinquaXD added a commit that referenced this issue Apr 18, 2024
# Description
First task of #2627

Creating contract instances with `ethcontract-rs` is very costly
(especially in tight loops).
A very significant amount of time is spent creating instances to query
allowances.


# Changes
Instead of creating 1 contract instance per token to fetch balances for
we now create a single erc20 instance and reuse it for all `allowance()`
calls. This is unfortunately pretty ugly since `ethcontract-rs` doesn't
really expose the generated calldata very nicely and doesn't really
compose with `web3`.

## How to test
Correctness should be covered by e2e tests
Performance was verified using the benchmark mentioned in
#2627

It's a bit hard to see but fetching allowances went from ~20% to ~7.6%
of the entire runtime.
The overall runtime of the axum request handler went from ~74% -> 63% of
the runtime.
Overall I think the performance gains are significant enough to warrant
the slightly ugly but very local performance hack until we replace
`ethcontract-rs` with `alloy` entirely.

Before
<img width="1728" alt="Screenshot 2024-04-17 at 21 32 46"
src="https://github.com/cowprotocol/services/assets/19190235/9d8d9cdf-ed72-4bff-a40b-c3cbf5418dbf">


Optimized
<img width="1728" alt="Screenshot 2024-04-17 at 21 28 35"
src="https://github.com/cowprotocol/services/assets/19190235/005aa2cd-7679-4d49-8220-c86cd25988a1">
Copy link

This issue has been marked as stale because it has been inactive a while. Please update this issue or it will be automatically closed.

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

No branches or pull requests

1 participant