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

[Memeland] App broken because max CPU cycles reached #1873

Closed
leohhhn opened this issue Apr 2, 2024 · 5 comments
Closed

[Memeland] App broken because max CPU cycles reached #1873

leohhhn opened this issue Apr 2, 2024 · 5 comments
Labels
🐞 bug Something isn't working

Comments

@leohhhn
Copy link
Contributor

leohhhn commented Apr 2, 2024

Description

Seems Memeland is broken because fetching its posts overruns the max CPU cycle count set in #1807. I can try lowering the amount of computation by optimizing the package/realm code, however I think doing so might improve the state a bit, but not so where it's functional again.

I need to demo the app on the 11th of April, we should fix this before then. What can we do about this?

cc @deelawn @zivkovicmilos

@zivkovicmilos
Copy link
Member

zivkovicmilos commented Apr 2, 2024

How many cycles does it usually need? For basic operations?

@zivkovicmilos zivkovicmilos added the 🐞 bug Something isn't working label Apr 2, 2024
@r3v4s
Copy link
Contributor

r3v4s commented Apr 5, 2024

Facing same issue for testing bunch of contracts.

Found max vm cycles is hardcoded here.

const (
maxAllocTx = 500 * 1000 * 1000
maxAllocQuery = 1500 * 1000 * 1000 // higher limit for queries
// maxVMCycles is the maximum number of cycles allowed while executing a single VM
// message. Ideally this should not be needed, as execution should halt when out of
// gas. The worst case scenario is that this value is used as a fallback.
maxVMCycles = 10_000_000
)

IMHO(as above comments) that '10_000_000' limit can be removed, as execution (will) panic if out of gas happens.

In this case we might need to set max gas per block higher (#1788)

@leohhhn
Copy link
Contributor Author

leohhhn commented Apr 8, 2024

Ok, investigated this case a bit. Here's what I found.
I am using the following image specs as a reference:

  • 1038 × 1000 px
  • 415KB
  1. PostMeme is cheap. it takes 346 CPU CYCLES to run with the above specs
  2. GetPostsInRange is super expensive - for this one image, it takes 1_665_086 CPU CYCLES to run. This means that at the current rate, Memeland can fetch about 7 memes at once before it breaks.

See how GetPostsInRange is implemented here.

The interesting thing is - the memeland UI uses QEval instead of MsgCall. I'm not sure how (or if) we handle gas usage / CPU cycles in case of a read-only call. cc @r3v4s & his issue regarding this here.

There are things I can do to try to improve the speed of the Memeland code, and can probably compromise on some features and try to optimize it (like cutting down maximum fetch to be 5 memes at once), but this situation ultimately shows us that:

  • images should probably not be stored on-chain
  • complicated operations, multiple loops, sorting, should not be done on-chain, at least at this point.

This app needs to be demoed on the 11th, so for now I suggest bumping the CPU cycles limit to 100_000_000, or reverting the PR that introduced it.

@ajnavarro
Copy link
Contributor

@leohhhn can we close this?

@leohhhn
Copy link
Contributor Author

leohhhn commented Apr 15, 2024

We reverted #1902 so I am closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants