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

Allow precache to clear in background #7438

Closed
wants to merge 5 commits into from
Closed

Conversation

benaadams
Copy link
Member

Changes

  • Allow prewarm cache to clear in background on prewarm continuation; only forcing at start of next block if not cleared

Types of changes

What types of changes does your code introduce?

  • Optimization

Testing

Requires testing

  • No

@@ -31,7 +31,7 @@ public Task PreWarmCaches(Block suggestedBlock, Hash256? parentStateRoot, Access
{
if (targetWorldState.ClearCache())
{
if (_logger.IsWarn) _logger.Warn("Caches are not empty. Clearing them.");
if (_logger.IsDebug) _logger.Debug("Caches are not empty. Clearing them.");
Copy link
Member

Choose a reason for hiding this comment

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

This generally should not happen in normal execution, why lower to Debug?

@@ -40,6 +40,12 @@ public static bool NoResizeClear<TKey, TValue>(this ConcurrentDictionary<TKey, T

using var handle = dictionary.AcquireLock();

// Recheck under lock
if (dictionary is null || dictionary.IsEmpty)
Copy link
Member

Choose a reason for hiding this comment

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

dictionary?.IsEmpty == false

Copy link
Member Author

Choose a reason for hiding this comment

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

dictionary?.IsEmpty == false

Crashes with that

dictionary?.IsEmpty ?? true works though

@@ -330,7 +330,6 @@ public void CommitTrees(long blockNumber)
_toUpdateRoots.Clear();
// only needed here as there is no control over cached storage size otherwise
_storages.Clear();
_preBlockCache?.NoResizeClear();
Copy link
Member

Choose a reason for hiding this comment

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

Not needed now?

@@ -857,7 +857,6 @@ public void CommitTree(long blockNumber)
}

_tree.Commit(blockNumber);
_preBlockCache?.NoResizeClear();
Copy link
Member

Choose a reason for hiding this comment

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

Not needed now?

@benaadams
Copy link
Member Author

Something not happy with this. Will make a simpler PR

@benaadams benaadams closed this Sep 16, 2024
@benaadams
Copy link
Member Author

Simpler that is kinda and optimiaztion bug fix #7442

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