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

fix: daily blockchain tests #1732

Merged
merged 1 commit into from
Jan 23, 2025
Merged

Conversation

DavePearce
Copy link
Collaborator

This fixes the issues running the daily blockchain tests, by doing two things:

  1. Updating to the latest version of go-corset, which includes some minor optimisations that help reduce cost.
  2. Adding a batching parameter for go-corset (-b 1024) which sets the batch size to max 1024. This throttles concurrency to some extent, and reduces memory pressure.

Overall, these changes appear to allow the daily blockchain tests to pass, and hopefully they will consistently pass going forward.

This version includes a small number of optimisations which may improve
overall performance.
@DavePearce DavePearce linked an issue Jan 23, 2025 that may be closed by this pull request
@DavePearce DavePearce enabled auto-merge (squash) January 23, 2025 09:13
@OlivierBBB OlivierBBB self-requested a review January 23, 2025 09:14
@OlivierBBB
Copy link
Collaborator

Approved. Where does the value 1024 come from and do you know how many batches the previously unthrottled implementation would use ? Also are these batches batches of columns that corset computes or something else ?

@DavePearce DavePearce merged commit b0a8026 into arith-dev Jan 23, 2025
12 checks passed
@DavePearce DavePearce deleted the 1723-fix-daily-blockchain-tests branch January 23, 2025 09:43
@DavePearce
Copy link
Collaborator Author

The 1024 figure is just one that seems to work well. I played around with a few. Without a batch size specified it tries to compute as many columns in one go as it can. Given that we have max ~2300
Columns, of which some are input columns and don't need to be computed ... i guess the largest batch probably isn't much larger than 1024. But the difference is obviously enough to tip it back into the green 🤔

@DavePearce
Copy link
Collaborator Author

Actually forget that. Its not columns causing the problem but constraints I guess. Which means it might be trying to do all 9000 in one go!! (Since constraints don't have dependencies between themselves)

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

Successfully merging this pull request may close these issues.

fix: daily blockchain tests
3 participants