Add benchmarks for fabtoken#1520
Conversation
9eeadbb to
acacfc4
Compare
|
Hey @AkramBitar , I fixed the lint issue. Can you please check it once. Thank You |
|
Hi @Rozerxshashank , great effort, really. I'll let @AkramBitar check this one to see if we have everything to have a comparison with the dlog driver. |
|
Hey @adecaro can you please help me with this golancli-lint/ lint(pull_request). I used co-pilot to explain the error and did as it said to do still the check is failing. Please guide me for it Thanks! 🙌 |
9f49b8a to
0bfeb86
Compare
|
@Rozerxshashank , run |
80133d0 to
204f045
Compare
|
Hi @Rozerxshashank , please, double-check the PR. There are files that should not be there. Thanks 🙏 |
5647973 to
42323f2
Compare
42323f2 to
e78eb89
Compare
Hello @Rozerxshashank thanks a lot for this great work and submitting this PR. I appreciate it a lot. |
e78eb89 to
c816cf1
Compare
|
Hi @AkramBitar, thanks for the feedback. I've updated the PR to fully align the fabtoken benchmarks with the 3-layer architecture from dlognogh. I added all missing Layer 1 and Layer 2 benchmarks (including Note: The CI failure in postgres storage tests is pre-existing on main and unrelated to these benchmark changes |
|
Hi @AkramBitar , please, can you have another look here? Thanks 🙏 |
Sure, will do this ASAP tomorrow. Thanks a lot. |
|
Hello @Rozerxshashank The PR looks great for me. Great work. Thanks a lot. |
c01eaf9 to
420aca7
Compare
|
@AkramBitar, I've added documentation. Please have a look. Thank You |
|
Hi @AkramBitar , please, can you have another pass on this PR. Thanks 🙏 |
Hello @Rozerxshashank |
Hello @Rozerxshashank, did you have the chance to work on this? |
41c931d to
bd5facf
Compare
HI @AkramBitar, sorry I missed this one earlier. Was working on other issues. I've addressed all your comments please have a look. Thanks! |
|
Thanks a lot @Rozerxshashank, One last thing. Did you have the chance to address my below comment? Could you please add link to these md file from: https://github.com/hyperledger-labs/fabric-token-sdk/blob/main/docs/drivers/benchmark/benchmark.md (as we did for ZKAT DLog)? |
bd5facf to
40589b2
Compare
Yes, sorry I misunderstood your request regarding the documentation links earlier. I have now updated the links in benchmark.md as requested. Thanks! |
|
Thanks a lot @Rozerxshashank for the great work. I appreciate it a lot. |
40589b2 to
b43f83a
Compare
Done! |
|
Hi @Rozerxshashank, I ran the following test and I got the following errors (almost for all tests) [FAIL] High Variance (CV 3183.60%). System noise is affecting results. Isolate the machine or increase duration. go test -bench=BenchmarkTransferProofGeneration -benchtime=10s Workers 32 Latency Distribution: Stability Metrics: System Health & Reliability: Latency Heatmap (Dynamic Range): --- Analysis & Recommendations ---
|
|
Hi @AkramBitar, Thanks for catching this. Yes, I reproduced the exact same errors locally. The issue stems from the default benchmark configuration. When To fix this permanently, I am introducing a bounded pooling strategy. When Running your benchmark command post-fix yields clean and stable throughput:
Does this sounds good to you? |
|
Hello @Rozerxshashank, Sorry for the delay. Thanks a lot for the above description. My question, why we count the setup time at all? I think we need to report the test performance without the setup time. Does this sounds good to you? go test -bench=BenchmarkTransferProofGeneration -benchtime=10s --- Analysis & Recommendations ---
|
|
Hi @AkramBitar, Thanks for getting back to me, and you definitely didn't miss anything. I completely agree that we should only measure the pure execution time without the setup overhead. The word "Setup" in the test name The reason you are still seeing the warnings is a mathematical false positive of the custom runner's strict metrics on high-speed operations. Because Fabtoken is so fast (averaging just 2 microseconds per operation), its latency baseline is incredibly low. Under these conditions, standard background operating system activities like thread context switching or CPU core migrations (which naturally take between 0.2ms and 2.0ms) get flagged as massive 100x to 300x latency spikes relative to the microsecond baseline. While the memory cap fix successfully resolved the GC thrashing and memory bloat, these false-positive warnings will always trigger under the runner's strict thresholds on a multi-core machine because the metrics were originally designed for slower zero-knowledge operations. |
@Rozerxshashank thanks a lot for the quick response. So based on what I understand the latency warnings are expected because Fabtoken operations are extremely fast (~2 microseconds). Normal OS scheduling and CPU context switching can take longer than the operation itself, causing false-positive latency spikes. Would it be better to measure performance in batches (e.g., thousands of operations per run) instead of single operations to reduce noise and get a more stable latency estimate? |
|
Hi @AkramBitar, That is an excellent suggestion, and batching is a very common industry standard to amortize scheduler and context-switching noise for microsecond-scale operations. If we wrap the work in batches of, say, 1000 operations inside the benchmark function, the execution time per measurement increases from 2 microseconds to around 2 milliseconds. This naturally suppresses the standard 0.1ms to 0.5ms OS scheduling noise, stabilizing the Coefficient of Variation (CV) and eliminating the false-positive spike warnings. The only small trade-off is that because the benchmark runner reports raw latency statistics based on the duration of each individual work call, the reported metrics (Min, Median, P99, Max) would represent the duration of the entire 1000-op batch instead of a single operation. We would just need to document the results as "latency per 1000 operations" or divide the reported numbers by 1000 to get the single-op latency. Would you prefer me to update the benchmark files to run in batches of 1000 so we can get stable, noise-resilient runs? |
Thanks a lot @Rozerxshashank Let's consult with @adecaro. |
|
yes, I would agree with the @Rozerxshashank 's analysis. The tool was not design for jobs that run so quickly. I think we are good to go here. We don't need to spend more time trying to optimizing the tool here. |
|
Hi @Rozerxshashank, Thanks a million for your work on that PR. Ready to go. Could you please rebase with main and squash all the commits? Regards, |
Signed-off-by: Shashank <yshashank959@gmail.com>
db9884c to
9c86fd6
Compare
Closes #1376
Overview
This PR implements a benchmarking suite for the FabToken driver. While the zkatdlog driver already had comprehensive performance tests, FabToken was missing equivalent coverage. These changes bring parity between the drivers and allow for consistent performance monitoring across the SDK.
Changes
I have added benchmarks for the core services of the FabToken V1 driver:
Performance Results
Initial testing on a local machine shows the following performance metrics for FabToken:
Verification
The new benchmarks have been verified using the project's standard benchmarking harness. I've also run go vet and gofmt on the updated files to ensure they comply with the project standards. The mock environments used in the tests were carefully configured to exercise the actual service logic while maintaining high performance.
The benchmarks leverage the existing utilities in token/services/benchmark to provide detailed metrics like throughput stability and latency distribution.