chore(gpu): write swap bench #2301
Conversation
There was a problem hiding this comment.
Reviewed 9 of 9 files at r1, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @tmontaigu)
.github/workflows/benchmark_gpu_dex_weekly.yml line 7 at r1 (raw file):
schedule: # Weekly benchmarks will be triggered each Saturday at 5a.m. - cron: '0 5 * * 6'
We are launching ERC20 benchmarks at the same moment, with the same type of instances. Could you delay this start by, let's say 2 hours ?
tfhe/benches/utilities.rs line 892 at r1 (raw file):
#[cfg(feature = "integer")] pub use cuda_integer_utils::*; use tfhe::{set_server_key, ClientKey, CompressedServerKey};
This use should be at the top of the module I think, just for clarity.
tfhe/benches/high_level_api/dex.rs line 59 at r1 (raw file):
where FheType: Add<Output = FheType> + for<'a> FheOrd<&'a FheType> + Clone, FheBool: IfThenElse<FheType>,
I don't see any reference to FheBool type in this function is it normal?
tfhe/benches/high_level_api/dex.rs line 746 at r1 (raw file):
} }
There no throughput benchmarks for swap_claim ?
There was a problem hiding this comment.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @soonum and @tmontaigu)
.github/workflows/benchmark_gpu_dex_weekly.yml line 7 at r1 (raw file):
Previously, soonum (David Testé) wrote…
We are launching ERC20 benchmarks at the same moment, with the same type of instances. Could you delay this start by, let's say 2 hours ?
Done.
tfhe/benches/utilities.rs line 892 at r1 (raw file):
Previously, soonum (David Testé) wrote…
This use should be at the top of the module I think, just for clarity.
Done.
tfhe/benches/high_level_api/dex.rs line 59 at r1 (raw file):
Previously, soonum (David Testé) wrote…
I don't see any reference to
FheBooltype in this function is it normal?
FheBool is not a generic type it's needed to use the IfThenElse trait in this case, because we use select in the transfer.
tfhe/benches/high_level_api/dex.rs line 746 at r1 (raw file):
Previously, soonum (David Testé) wrote…
There no throughput benchmarks for
swap_claim?
I have not implemented it for now, actually I'm not even sure we need the throughput bench for the request I was hesitating to add it 🤔
There was a problem hiding this comment.
Reviewed 2 of 2 files at r2.
Reviewable status: all files reviewed (commit messages unreviewed), 2 unresolved discussions (waiting on @agnesLeroy and @tmontaigu)
tfhe/benches/high_level_api/dex.rs line 746 at r1 (raw file):
Previously, agnesLeroy (Agnès Leroy) wrote…
I have not implemented it for now, actually I'm not even sure we need the throughput bench for the request I was hesitating to add it 🤔
Should we go for less code then ? And add throughput benchmarks for both request and claim when they are needed ?
|
Previously, soonum (David Testé) wrote…
Ok let's do that, I'll remove it for now. |
|
Previously, agnesLeroy (Agnès Leroy) wrote…
Done |
There was a problem hiding this comment.
Reviewed 1 of 1 files at r3.
Reviewable status: 8 of 9 files reviewed, all discussions resolved (waiting on @tmontaigu)
closes: please link all relevant issues
PR content/description
This PR introduces a new HL API benchmark for the Swap operation, which is executed in the DEX contract.
There are 2 phases in a confidential swap: the request and the claim. Rand described the flow as:
We technically only need to run in FHE the deposit and the claim, the rest is not FHE. We also don't care about having more than 1 user in a batch, so we can benchmark with only 1 deposit and 1 claim.
FHEVM implementation:
I have included a throughput benchmark for the request but maybe it's not necessary.
I have checked with JAT that the Rust implementation corresponds to what's done in Solidity. The claim requires executing 128-bit scalar mul & scalar division for contracts operating on 64-bit encrypted integers.
Check-list:
This change is