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

Perf/faster full tree visit #7692

Merged
merged 11 commits into from
Oct 31, 2024
Merged

Perf/faster full tree visit #7692

merged 11 commits into from
Oct 31, 2024

Conversation

asdacap
Copy link
Contributor

@asdacap asdacap commented Oct 31, 2024

  • Improved tree visitor core utilization.
  • On R9 7950x Improved read throughput by 2.5x. With CPU utilization around 80-90%.
  • When combined with Feature/verify trie on state finish #7691 (there is a code cache there), can finish mainnet verify trie in 4 minute 30 second.

Screenshot from 2024-10-31 12-28-45

Types of changes

What types of changes does your code introduce?

  • Optimization

Testing

Requires testing

  • Yes

If yes, did you write tests?

  • Yes

Notes on testing

  • Tested manually on a mainnet backup.

@asdacap asdacap mentioned this pull request Oct 31, 2024
5 tasks
src/Nethermind/Nethermind.Core/Threading/ThreadLimiter.cs Outdated Show resolved Hide resolved
Comment on lines 24 to 27
int newSlot = Interlocked.Decrement(ref _slots);
if (newSlot < 0)
{
Interlocked.Increment(ref _slots);
Copy link
Member

Choose a reason for hiding this comment

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

Can this be done better with voliatale? @Scooletz

Copy link
Contributor

@Scooletz Scooletz Oct 31, 2024

Choose a reason for hiding this comment

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

It can't easily as you want to undo the previous Decrement in an atomic way. You'll need at least one CAS to get it. The case here though is that on faults, you do two CAS operations. In such scenarios you could

var counter = Volatile.Read(ref _slots)
if (counter <= 1)
{
   return false;
}
if (Interlocked.CompareExchange(ref slots, counter -1, counter) == counter))
{
  return true;
}

// slow path in the contended scenario, with loop retry

The question is how contended it is and how likely it's to hit the condition of returning false.

src/Nethermind/Nethermind.Core/Threading/ThreadLimiter.cs Outdated Show resolved Hide resolved
});
}

if (tasks != null && tasks.Count > 0) Task.WaitAll(tasks.ToArray());
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (tasks != null && tasks.Count > 0) Task.WaitAll(tasks.ToArray());
if tasks?.Count > 0) Task.WaitAll(tasks.ToArray());

Also we can use do AsSpan() instead of ToArray in .net 9 (update #7585 when this is merged to master)

asdacap and others added 2 commits October 31, 2024 16:15
Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com>
Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com>
asdacap and others added 4 commits October 31, 2024 17:08
@asdacap asdacap merged commit 159c2a4 into master Oct 31, 2024
75 checks passed
@asdacap asdacap deleted the perf/faster-full-table-scan branch October 31, 2024 14:54
@kamilchodola kamilchodola restored the perf/faster-full-table-scan branch November 4, 2024 13:43
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.

4 participants