Skip to content

Clean up memories when syncing #1585

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 11, 2019
Merged

Clean up memories when syncing #1585

merged 1 commit into from
Jun 11, 2019

Conversation

@HoOngEe
Copy link
Contributor

@HoOngEe HoOngEe commented May 30, 2019 •
edited
Loading

The commits in this pr piled upon the #1567
It closes #1576
@sgkim126
Copy link
Contributor

sgkim126 commented May 30, 2019

@HoOngEe Would you shrink targets too?
@HoOngEe
Copy link
Contributor Author

HoOngEe commented May 30, 2019

Before shrinking targets the memory usage reduced from about 9.0GB to 5.3GB.
@HoOngEe
Copy link
Contributor Author

HoOngEe commented May 31, 2019

I ran memory profiling tool valgrin --tool=massif, used massif-visualizer and got the following graph.
memory_profile

This time I got data of only 68000 blocks synchronization. So I cannot certain that the data from these two key parts: header_queue of importer, downloaded of HeaderDownloader would be shrunk later. The running time increased about 20 times before attaching heap memory profiler so that I cannot observe in full-time domain. Now I'll check header_queue and body_queue
@HoOngEe
Copy link
Contributor Author

HoOngEe commented May 31, 2019 •
edited
Loading

We have to shrink processing: RwLock<HashMap<H256, U256>> in VerificationQueue also.
sgkim126
sgkim126 reviewed May 31, 2019
View reviewed changes
@sgkim126 sgkim126 added the sync label May 31, 2019
@HoOngEe HoOngEe force-pushed the fix/uncleared-memory branch 3 times, most recently from 4147c49 to 134c094 Compare May 31, 2019 06:38
@HoOngEe
Copy link
Contributor Author

HoOngEe commented Jun 3, 2019

I got a full-time synchronization data through last weekends.
full-time

The purple area, which was allocated in the block_header_data of HeaderChain implementing HeaderProvider, accounts for almost memory uncleared even after synchronizing old blocks.
It uses HashMap named header_cache internally. But there is no entry clearing routine for this.
@sgkim126
Copy link
Contributor

sgkim126 commented Jun 5, 2019

@HoOngEe I know still there is a memory to cleaning up. However, I think it's hard to fix the problem, there is no apparent time to clean the cache. It should change the implementation to LRU like implementation.
So I propose to merge this PR and keep #1576 open until we fix the memory issue completely.
How about you?
@HoOngEe
Copy link
Contributor Author

HoOngEe commented Jun 10, 2019

@sgkim126 Ok, let's merge this pr now and fix the cache implementation later.
sgkim126
sgkim126 reviewed Jun 10, 2019
View reviewed changes
Copy link
Contributor

@sgkim126 sgkim126 left a comment

Choose a reason for hiding this comment

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

Please remove WIP tag when you think it's ready to merge.
@HoOngEe HoOngEe changed the title [WIP] Clean up memories when syncing Clean up memories when syncing Jun 10, 2019
sgkim126
sgkim126 requested changes Jun 10, 2019
View reviewed changes
core/src/verification/queue/mod.rs Outdated
*td -= score;
}
}
processing.shrink_to_fit();
Copy link
Contributor

@sgkim126 sgkim126 Jun 10, 2019

Choose a reason for hiding this comment

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

This seems redundant.
Copy link
Contributor Author

@HoOngEe HoOngEe Jun 11, 2019

Choose a reason for hiding this comment

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

Yes, It's redundant. I fixed it. Thank you.
@HoOngEe HoOngEe force-pushed the fix/uncleared-memory branch from 29c49c8 to 603c211 Compare June 11, 2019 07:06
@HoOngEe
Make the sync module release memory aggressively
0f4cded 
Shrink HashMaps and Vectors of downloaders and Verification queue to reduce the memory usage when syncing.
@HoOngEe HoOngEe force-pushed the fix/uncleared-memory branch from 603c211 to 0f4cded Compare June 11, 2019 07:07
@sgkim126
Copy link
Contributor

sgkim126 commented Jun 11, 2019

@HoOngEe LGTM. Please apply it to rc-1.4.0 too.
@mergify mergify bot merged commit 9c4c362 into CodeChain-io:master Jun 11, 2019
@HoOngEe HoOngEe mentioned this pull request Aug 28, 2019
Closed
@HoOngEe HoOngEe deleted the fix/uncleared-memory branch August 28, 2019 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sync

2 participants

@HoOngEe @sgkim126