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

Change Merkle Tree to linear data structure #7

Closed
wants to merge 19 commits into from
Closed

Conversation

dloghin
Copy link
Collaborator

@dloghin dloghin commented Feb 28, 2024

The original data structure for Merkle Tree in Plonky2 is recursive. This PR replaces this recursive data structure with a linear (typical binary tree array representation).

@dloghin dloghin requested a review from cliff0412 February 28, 2024 09:28
print_time(now, "fill init");
let now = Instant::now();

// copy data to C
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could use rust to copy directly?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fill_digests_buf_gpu_v2() copies directly from Rust to the GPU, but the performance is lower compared to Rust -> C -> GPU. We need to investigate the reason

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed this, now the performance is almost the same (maybe only 1ms difference).

@cliff0412
Copy link
Collaborator

cliff0412 commented Feb 29, 2024

suggest to reuse the data after lde inside GPU in cases where LDE is before merkle tree. or maybe we could use separate features or different size of data to control whether use LDE. currently, LDE is applied when log_n is > 10 (where gpu is faster). when log_n < 10, it will fall back to cpu code

@dloghin dloghin closed this Mar 15, 2024
cliff0412 pushed a commit that referenced this pull request Mar 18, 2024
* Remove values of last memory channel

Co-authored-by: Linda Guiga <[email protected]>

* Fix merge

* Apply comments

* Fix ASM

* Top stack documentation (#7)

* Add doc file

* Apply comments

* Apply comments

* Fix visibility

* Fix visibility

---------

Co-authored-by: Linda Guiga <[email protected]>
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.

2 participants