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

jamtis: implement dense/sparse view privacy changes #15

Closed

Conversation

jeffro256
Copy link

@jeffro256 jeffro256 marked this pull request as draft September 6, 2023 02:43
@jeffro256 jeffro256 force-pushed the jamtis_fix_fr_privacy branch 2 times, most recently from 49e2409 to 7d888fc Compare September 6, 2023 20:31
@jeffro256 jeffro256 changed the title jamtis: implement find-received privacy changes [WIP] jamtis: implement find-received privacy changes Sep 6, 2023
@jeffro256 jeffro256 changed the title jamtis: implement find-received privacy changes jamtis: implement dense/sparse view privacy changes Sep 6, 2023
@jeffro256 jeffro256 marked this pull request as ready for review September 6, 2023 20:38
Copy link
Owner

@UkoeHB UkoeHB left a comment

Choose a reason for hiding this comment

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

Just skimmed it for now. I recommend running comparative perf tests for scanning before/after this PR and adding them to the PR comments.

tests/performance_tests/view_scan.h Outdated Show resolved Hide resolved
@jeffro256
Copy link
Author

Here's the results of test_remote_scanner_client_scan_sp performance test after fixing it:

seraphis_lib (original) run #1
test_remote_scanner_client_scan_sp (23 calls) - OK: 13434 µs/call
test_remote_scanner_client_scan_sp (23 calls) - OK: 13217 µs/call
test_remote_scanner_client_scan_sp (23 calls) - OK: 14304 µs/call

seraphis_lib (original) run #2
test_remote_scanner_client_scan_sp (23 calls) - OK: 13086 µs/call
test_remote_scanner_client_scan_sp (23 calls) - OK: 13130 µs/call
test_remote_scanner_client_scan_sp (23 calls) - OK: 14217 µs/call

jamtis_fix_fr_privacy (new PR) run #1
test_remote_scanner_client_scan_sp (23 calls) - OK: 1738 ms/call
test_remote_scanner_client_scan_sp (23 calls) - OK: 1738 ms/call
test_remote_scanner_client_scan_sp (23 calls) - OK: 1739 ms/call

jamtis_fix_fr_privacy (new PR) run #2
test_remote_scanner_client_scan_sp (23 calls) - OK: 1731 ms/call
test_remote_scanner_client_scan_sp (23 calls) - OK: 1750 ms/call
test_remote_scanner_client_scan_sp (23 calls) - OK: 1734 ms/call

Ran on an AMD Ryzen 9 3900X 12-core CPU. So processing basic records from a remote scanner is ~128x slower (about what was predicted from Tevador's cycle estimates).

@jeffro256
Copy link
Author

One performance difference that works in the new changes favor, though, is that basic records are 9% smaller because the nominal address index is not transmitted.

Results of `test_remote_scanner_client_scan_sp` performance test:
```
seraphis_lib (original) run #1
test_remote_scanner_client_scan_sp (23 calls) - OK: 13434 µs/call
test_remote_scanner_client_scan_sp (23 calls) - OK: 13217 µs/call
test_remote_scanner_client_scan_sp (23 calls) - OK: 14304 µs/call

seraphis_lib (original) run #2
test_remote_scanner_client_scan_sp (23 calls) - OK: 13086 µs/call
test_remote_scanner_client_scan_sp (23 calls) - OK: 13130 µs/call
test_remote_scanner_client_scan_sp (23 calls) - OK: 14217 µs/call

jamtis_fix_fr_privacy (new PR) run #1
test_remote_scanner_client_scan_sp (23 calls) - OK: 1738 ms/call
test_remote_scanner_client_scan_sp (23 calls) - OK: 1738 ms/call
test_remote_scanner_client_scan_sp (23 calls) - OK: 1739 ms/call

jamtis_fix_fr_privacy (new PR) run #2
test_remote_scanner_client_scan_sp (23 calls) - OK: 1731 ms/call
test_remote_scanner_client_scan_sp (23 calls) - OK: 1750 ms/call
test_remote_scanner_client_scan_sp (23 calls) - OK: 1734 ms/call
```

Ran on an AMD Ryzen 9 3900X 12-core CPU. Processing basic records from a remote scanner is about 128x slower.
@UkoeHB
Copy link
Owner

UkoeHB commented Sep 10, 2023

If my math is right, the updated scanning for a scanner client is 1s per 10mill on-chain enotes (40k are scanned locally). 40k enotes transmitted for local scanning is at least 5-10MB so the overall cost is comparable with modern download speeds.

@jeffro256
Copy link
Author

Closing in favor of #26

@jeffro256 jeffro256 closed this Dec 3, 2023
@jeffro256 jeffro256 mentioned this pull request Dec 6, 2023
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