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

perf: reduce sumcheck memory usage #430

Merged
merged 5 commits into from
Jan 31, 2025
Merged

Conversation

JayWhite2357
Copy link
Contributor

Rationale for this change

The sumcheck prover currently consumes a lot of memory.

What changes are included in this PR?

See individual commits. Broadly speaking, this is just a refactor of make_sumcheck_prover_state, so it is fairly isolated.

Are these changes tested?

Yes

@JayWhite2357 JayWhite2357 force-pushed the refactor/sumcheck-optimization branch from 80216d5 to c457b9a Compare December 12, 2024 04:20
@JayWhite2357 JayWhite2357 marked this pull request as draft January 31, 2025 13:52
@JayWhite2357 JayWhite2357 removed the request for review from jacobtrombetta January 31, 2025 13:52
@iajoiner iajoiner force-pushed the refactor/sumcheck-optimization branch from c457b9a to 846be50 Compare January 31, 2025 15:16
@iajoiner iajoiner self-requested a review January 31, 2025 15:16
@iajoiner iajoiner marked this pull request as ready for review January 31, 2025 15:17
@iajoiner iajoiner merged commit 586f714 into main Jan 31, 2025
13 checks passed
Copy link

🎉 This PR is included in version 0.71.6 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@iajoiner iajoiner mentioned this pull request Jan 31, 2025
8 tasks
iajoiner added a commit that referenced this pull request Feb 5, 2025
Please be sure to look over the pull request guidelines here:
https://github.com/spaceandtimelabs/sxt-proof-of-sql/blob/main/CONTRIBUTING.md#submit-pr.
Depends on the following PRs:
- [x] #430
- [x] #482 
- [x] #503
- [x] #506
- [x] #509 
- [x] #510

Closes #394.
# Please go through the following checklist
- [x] The PR title and commit messages adhere to guidelines here:
https://github.com/spaceandtimelabs/sxt-proof-of-sql/blob/main/CONTRIBUTING.md.
In particular `!` is used if and only if at least one breaking change
has been introduced.
- [x] I have run the ci check script with `source
scripts/run_ci_checks.sh`.

# Rationale for this change
We need to add sort merge inner joins.
<!--
Why are you proposing this change? If this is already explained clearly
in the linked issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand
your changes and offer better suggestions for fixes.

 Example:
 Add `NestedLoopJoinExec`.
 Closes #345.

Since we added `HashJoinExec` in #323 it has been possible to do
provable inner joins. However performance is not satisfactory in some
cases. Hence we need to fix the problem by implement
`NestedLoopJoinExec` and speed up the code
 for `HashJoinExec`.
-->

# What changes are included in this PR?
- add `SortMergeJoinExec`
- add `Column::rho` and `Table::add_rho_column`
<!--
There is no need to duplicate the description in the ticket here but it
is sometimes worth providing a summary of the individual changes in this
PR.

Example:
- Add `NestedLoopJoinExec`.
- Speed up `HashJoinExec`.
- Route joins to `NestedLoopJoinExec` if the outer input is sufficiently
small.
-->

# Are these changes tested?
<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code

If tests are not included in your PR, please explain why (for example,
are they covered by existing tests)?

Example:
Yes.
-->
Will be.
@iajoiner iajoiner deleted the refactor/sumcheck-optimization branch February 6, 2025 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants