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

v1.18: accounts-db: fix 8G+ memory spike during hash calculation (backport of #1308) #1318

Merged
merged 1 commit into from
Jun 17, 2024

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented May 13, 2024

We were accidentally doing several thousands 4MB allocations - even during incremental hash - which added up to a 8G+ memory spikes over ~2s every ~30s.

Fix by using Vec::new() in the identity function. Empirically 98%+ reduces join arrays with less than 128 elements, and only the last few reduces join large vecs. Because realloc does exponential growth we don't see pathological reallocation but reduces do at most one realloc (and often 0 because of exp growth).


This is an automatic backport of pull request #1308 done by Mergify.

@mergify mergify bot requested review from a team as code owners May 13, 2024 14:46
@alessandrod
Copy link

Why I want to backport this: it's a one line change, and it's not hard to reason about. We've seen big stake nodes struggle especially badly with jemalloc stopping random threads to release memory. This removes a 8G alloc/dealloc spike, and I wouldn't be surprised if this fix alone improved skip rate on those nodes.

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 80.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 81.6%. Comparing base (4b7d0c2) to head (c9417f1).

Additional details and impacted files
@@           Coverage Diff           @@
##            v1.18    #1318   +/-   ##
=======================================
  Coverage    81.6%    81.6%           
=======================================
  Files         827      827           
  Lines      225432   225434    +2     
=======================================
+ Hits       184067   184071    +4     
+ Misses      41365    41363    -2     

We were accidentally doing several thousands 4MB allocations - even
during incremental hash - which added up to a 8G+ memory spikes over ~2s
every ~30s.

Fix by using Vec::new() in the identity function. Empirically 98%+
reduces join arrays with less than 128 elements, and only the last few
reduces join large vecs. Because realloc does exponential growth we
don't see pathological reallocation but reduces do at most one realloc
(and often 0 because of exp growth).

(cherry picked from commit 2c71685)
@willhickey willhickey merged commit c027cfc into v1.18 Jun 17, 2024
35 checks passed
@willhickey willhickey deleted the mergify/bp/v1.18/pr-1308 branch June 17, 2024 19:01
anwayde pushed a commit to firedancer-io/agave that referenced this pull request Jul 23, 2024
…kport of anza-xyz#1308) (anza-xyz#1318)

accounts-db: fix 8G+ memory spike during hash calculation (anza-xyz#1308)

We were accidentally doing several thousands 4MB allocations - even
during incremental hash - which added up to a 8G+ memory spikes over ~2s
every ~30s.

Fix by using Vec::new() in the identity function. Empirically 98%+
reduces join arrays with less than 128 elements, and only the last few
reduces join large vecs. Because realloc does exponential growth we
don't see pathological reallocation but reduces do at most one realloc
(and often 0 because of exp growth).

(cherry picked from commit 2c71685)

Co-authored-by: Alessandro Decina <alessandro.d@gmail.com>
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.

5 participants