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

soroban-rpc: Cache all ledger entries queried from DB in read transaction #845

Merged
merged 4 commits into from
Aug 8, 2023

Conversation

2opremio
Copy link
Contributor

@2opremio 2opremio commented Aug 7, 2023

What

As a followup to #837 , also cache all entries queried from the DB in a read transaction.

To have finer control over caching, a new contructor (NewCachedTx) has been added. This allows disabling caching in situations in which the cache size could grow out of hand.

Why

The preflight library (which in turn depends on rs-env-host) sometimes queries the same entry more than once.

Result

The performance improvement is not huge, but it allows us to be more flexible on the preflight library, not needing to be careful about querying the same entry multiple times.

Also note that the benchmark is pretty naive and doesn't query a lot of ledger entries from the DB.

Before:

goos: darwin
goarch: arm64
pkg: github.com/stellar/soroban-tools/cmd/soroban-rpc/internal/preflight
BenchmarkGetPreflight
BenchmarkGetPreflight/In-memory_storage
BenchmarkGetPreflight/In-memory_storage-12         	    5709	    211948 ns/op
BenchmarkGetPreflight/DB_storage
BenchmarkGetPreflight/DB_storage-12                	    3009	    383341 ns/op
PASS

After:

goos: darwin
goarch: arm64
pkg: github.com/stellar/soroban-tools/cmd/soroban-rpc/internal/preflight
BenchmarkGetPreflight
BenchmarkGetPreflight/In-memory_storage
BenchmarkGetPreflight/In-memory_storage-12         	    5658	    207791 ns/op
BenchmarkGetPreflight/DB_storage
BenchmarkGetPreflight/DB_storage-12                	    3591	    315048 ns/op
PASS

Known limitations

N/A

Copy link
Contributor

@tamirms tamirms left a comment

Choose a reason for hiding this comment

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

this will not cache lookups for entries which don't exist. so if we lookup the same key which is not present we will still repeat the sqlite queries. if this is ok then the PR looks good to me!

@2opremio
Copy link
Contributor Author

2opremio commented Aug 8, 2023

this will not cache lookups for entries which don't exist.

Uhm, good point, maybe I should fix this.

@2opremio
Copy link
Contributor Author

2opremio commented Aug 8, 2023

I fixed it and now the cache performs even better:

goos: darwin
goarch: arm64
pkg: github.com/stellar/soroban-tools/cmd/soroban-rpc/internal/preflight
BenchmarkGetPreflight
BenchmarkGetPreflight/In-memory_storage
BenchmarkGetPreflight/In-memory_storage-12         	    6151	    194964 ns/op
BenchmarkGetPreflight/DB_storage
BenchmarkGetPreflight/DB_storage-12                	    4008	    274884 ns/op
PASS

we are only 0.7 ms (40% more) away from the in-memory storage.

@2opremio 2opremio merged commit 731786e into stellar:main Aug 8, 2023
@2opremio 2opremio deleted the cache-in-read-tx branch August 8, 2023 21:11
kalepail pushed a commit to kalepail/soroban-tools that referenced this pull request Aug 10, 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