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

feat: enable receipt prefetching by default #7661

Merged
merged 2 commits into from
Sep 23, 2022

Conversation

jakmeier
Copy link
Contributor

Prefetch receipt meta data (account and access keys) ahead of time.
This recent performance optimization has been disabled by default.

In lab settings, performance improvement is confirmed. Using the
estimator to measure the time it takes to process empty receipts,
on a DB with 2 million accounts, on a local SSD, with enabled shard
caches. The result is as follows.

  • sender = receiver: 737us -> 386 us
  • sender != receiver: 1014us -> 644us
  • overhead per block: 6.9us -> 7.4us

Note that this is with 100 empty receipts in the same block, with all
different accounts. In real traffic it usually does not happen that so
many different accounts are accessed in the same block. But it is
allowed and we must be able process this case in reasonable time.
So even if it might not help in the average case, it makes sense to
activate this feature to speed up the worst-case.

Currently we use 8 IO threads per shard. Repeated experiments with
more threads showed no difference.
Decreasing it to 4 threads is about equal to 8 threads. Going lower is
significantly worse. Thus, overall, 8 threads seems reasonable here.

Canary nodes in testnet and mainnet with the feature enabled show that
the feature also works as expected on real traffic. The memory impact is
minimal, usually less than 40MB of reserved capacity, which is less than
8MB actual memory because 8 threads reserve 4MB each ahead of actually
fetching the data.

@jakmeier jakmeier requested a review from a team as a code owner September 21, 2022 16:53
@jakmeier
Copy link
Contributor Author

@mm-near nominating you as a reviewer since you already reviewed the feature, this one is just the decision to activate it by default from now on

core/store/src/config.rs Show resolved Hide resolved
Prefetch receipt meta data (account and access keys) ahead of time.
This recent performance optimization has been disabled by default.

In lab settings, performance improvement is confirmed. Using the
estimator to measure the time it takes to process empty receipts,
on a DB with 2 million accounts, on a local SSD, with enabled shard
caches. The result is as follows.

- sender = receiver:  737us -> 386 us
- sender != receiver: 1014us -> 644us
- overhead per block: 6.9us -> 7.4us

Note that this is with 100 empty receipts in the same block, with all
different accounts. In real traffic it usually does not happen that so
many different accounts are accessed in the same block. But it is
allowed and we must be able process this case in reasonable time.
So even if it might not help in the average case, it makes sense to
activate this feature to speed up the worst-case.

Currently we use 8 IO threads per shard. Repeated experiments with
more threads showed no difference.
Decreasing it to 4 threads is about equal to 8 threads. Going lower is
significantly worse. Thus, overall, 8 threads seems reasonable here.

Canary nodes in testnet and mainnet with the feature enabled show that
the feature also works as expected on real traffic. The memory impact is
minimal, usually less than 40MB of reserved capacity, which is less than
8MB actual memory because 8 threads reserve 4MB each ahead of actually
fetching the data.
@near-bulldozer near-bulldozer bot merged commit 1def1e9 into near:master Sep 23, 2022
jakmeier added a commit to jakmeier/nearcore that referenced this pull request Sep 26, 2022
Explicitly stop and wait for prefetching background threads to terminate
when a testbed is dropped. This avoids that estimations are influenced
by background threads left over from previous estimations, which we have
observed since merging near#7661.
@jakmeier jakmeier deleted the enable-receipt-prefetch branch September 26, 2022 12:32
nikurt pushed a commit that referenced this pull request Sep 26, 2022
Prefetch receipt meta data (account and access keys) ahead of time.
This recent performance optimization has been disabled by default.

In lab settings, performance improvement is confirmed. Using the
estimator to measure the time it takes to process empty receipts,
on a DB with 2 million accounts, on a local SSD, with enabled shard
caches. The result is as follows.

- sender = receiver:  737us -> 386 us
- sender != receiver: 1014us -> 644us
- overhead per block: 6.9us -> 7.4us

Note that this is with 100 empty receipts in the same block, with all
different accounts. In real traffic it usually does not happen that so
many different accounts are accessed in the same block. But it is
allowed and we must be able process this case in reasonable time.
So even if it might not help in the average case, it makes sense to
activate this feature to speed up the worst-case.

Currently we use 8 IO threads per shard. Repeated experiments with
more threads showed no difference.
Decreasing it to 4 threads is about equal to 8 threads. Going lower is
significantly worse. Thus, overall, 8 threads seems reasonable here.

Canary nodes in testnet and mainnet with the feature enabled show that
the feature also works as expected on real traffic. The memory impact is
minimal, usually less than 40MB of reserved capacity, which is less than
8MB actual memory because 8 threads reserve 4MB each ahead of actually
fetching the data.
jakmeier added a commit to jakmeier/nearcore that referenced this pull request Sep 28, 2022
Explicitly stop and wait for prefetching background threads to terminate
when the `ShardTriesInner` is dropped.

 This avoids that estimations are influenced by background threads left
 over from previous estimations, which we have observed since merging
 near#7661.
near-bulldozer bot pushed a commit that referenced this pull request Sep 29, 2022
Explicitly stop and wait for prefetching background threads to terminate when the `ShardTriesInner` is dropped.

 This avoids that estimations are influenced by background threads left
 over from previous estimations, which we have observed since merging
 #7661.
nikurt pushed a commit that referenced this pull request Nov 9, 2022
Prefetch receipt meta data (account and access keys) ahead of time.
This recent performance optimization has been disabled by default.

In lab settings, performance improvement is confirmed. Using the
estimator to measure the time it takes to process empty receipts,
on a DB with 2 million accounts, on a local SSD, with enabled shard
caches. The result is as follows.

- sender = receiver:  737us -> 386 us
- sender != receiver: 1014us -> 644us
- overhead per block: 6.9us -> 7.4us

Note that this is with 100 empty receipts in the same block, with all
different accounts. In real traffic it usually does not happen that so
many different accounts are accessed in the same block. But it is
allowed and we must be able process this case in reasonable time.
So even if it might not help in the average case, it makes sense to
activate this feature to speed up the worst-case.

Currently we use 8 IO threads per shard. Repeated experiments with
more threads showed no difference.
Decreasing it to 4 threads is about equal to 8 threads. Going lower is
significantly worse. Thus, overall, 8 threads seems reasonable here.

Canary nodes in testnet and mainnet with the feature enabled show that
the feature also works as expected on real traffic. The memory impact is
minimal, usually less than 40MB of reserved capacity, which is less than
8MB actual memory because 8 threads reserve 4MB each ahead of actually
fetching the data.
nikurt pushed a commit that referenced this pull request Nov 9, 2022
Explicitly stop and wait for prefetching background threads to terminate when the `ShardTriesInner` is dropped.

 This avoids that estimations are influenced by background threads left
 over from previous estimations, which we have observed since merging
 #7661.
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