Created dummy entry generator for benchmark #273
Conversation
kzg_prover/benches/kzg.rs
Outdated
|
|
||
| // Demonstrating that a higher value of K has a more significant impact on benchmark performance than the number of users | ||
| #[cfg(not(feature = "no_range_check"))] | ||
| { | ||
| const K: u32 = 18; | ||
| const N_USERS: usize = 16; | ||
| const N_USERS: usize = (1 << 17) + (1 << 16) - 6; |
There was a problem hiding this comment.
Can you try to replace it with 2^K-6?
There was a problem hiding this comment.
And if it works, apply it everywhere throughout the benchmark.
There was a problem hiding this comment.
Indeed, that is worked. Similarly, we can apply this to N_POINTS as follows:
const N_POINTS = N_CURRENCIES + 1;
kzg_prover/benches/kzg.rs
Outdated
|
|
||
| // Demonstrating that a higher value of K has a more significant impact on benchmark performance than the number of users | ||
| #[cfg(not(feature = "no_range_check"))] | ||
| { | ||
| const K: u32 = 18; | ||
| const N_USERS: usize = 16; | ||
| const N_USERS: usize = (1 << 17) + (1 << 16) - 6; |
There was a problem hiding this comment.
And if it works, apply it everywhere throughout the benchmark.
kzg_prover/benches/kzg.rs
Outdated
| @@ -59,7 +58,7 @@ fn bench_kzg< | |||
|
|
|||
| let mut entries: Vec<Entry<N_CURRENCIES>> = vec![Entry::init_empty(); N_USERS]; | |||
| let mut cryptos = vec![Cryptocurrency::init_empty(); N_CURRENCIES]; | |||
| parse_csv_to_entries::<&str, N_CURRENCIES>(csv_path, &mut entries, &mut cryptos).unwrap(); | |||
| let _ = generate_dummy_entries(&mut entries, &mut cryptos); | |||
There was a problem hiding this comment.
I think let _ is unnecessary.
There was a problem hiding this comment.
It is for warning, but this one would be fix with for #273 (comment)
|
|
||
| // This is for testing purposes with a large dataset instead of using a CSV file | ||
| pub fn generate_dummy_entries<const N_CURRENCIES: usize>( | ||
| entries: &mut [Entry<N_CURRENCIES>], |
There was a problem hiding this comment.
In this case it should be ok to return the (entries, currencies) from the function instead of passing in and filling the mutable arrays (which is used in parser to save the memory).
kzg_prover/benches/kzg.rs
Outdated
|
|
||
| // Demonstrating that a higher value of K has a more significant impact on benchmark performance than the number of users | ||
| #[cfg(not(feature = "no_range_check"))] | ||
| { | ||
| const K: u32 = 18; | ||
| const N_USERS: usize = 16; | ||
| const N_USERS: usize = 2usize.pow(K - 1) + 2usize.pow(K - 2) - 6; // Which is equal to (1 << 17) + (1 << 16) - 6 |
There was a problem hiding this comment.
Can you explain the value 2^(K-1)-2^(K-2)-6? Why in the case of K=17 you're using 2^(K-1)-6?
There was a problem hiding this comment.
I think the correct max value should be 2^K - 6.
There was a problem hiding this comment.
I'm unable to provide a clear explanation for the failure of the
There was a problem hiding this comment.
This has led me to believe that the maximum value for N_USER might be within the range between
|
Updated the maximum number of entries at here 3489dd1 |
N_USERS, instead of relying on entry data from a CSV file, which constrains us to a fixed user count.N_USERSin the benchmark code,kzg.rswith maximum number of user within givenKN_ASSETStoN_CURRENCIES