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

[Merged by Bors] - Enable malloc metrics for the VC #3279

Closed
wants to merge 2 commits into from

Conversation

michaelsproul
Copy link
Member

@michaelsproul michaelsproul commented Jun 20, 2022

Issue Addressed

Following up from #3223 (comment), it has been observed that the validator client uses vastly more memory in some compilation configurations than others. Compiling with Cross and then putting the binary into an Ubuntu 22.04 image seems to use 3x more memory than compiling with Cargo directly on Debian bullseye.

Proposed Changes

Enable malloc metrics for the validator client. This will hopefully allow us to see the difference between the two compilation configs and compare heap fragmentation. This PR doesn't enable malloc tuning for the VC because it was found to perform significantly worse. The --disable-malloc-tuning flag is repurposed to just disable the metrics.

@michaelsproul michaelsproul added ready-for-review The code is ready for review low-hanging-fruit Easy to resolve, get it before someone else does! labels Jun 20, 2022
Copy link
Member

@macladson macladson left a comment

Choose a reason for hiding this comment

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

LGTM!

@michaelsproul
Copy link
Member Author

free-mem

res

The malloc tuning seems to have made things slightly worse on our Prater nodes. I'll continue monitoring it for a bit, and if it keeps creeping up I'll change this PR so that it enables the malloc metrics but not the tuning.

@michaelsproul michaelsproul added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Jun 20, 2022
@michaelsproul michaelsproul changed the title Enable malloc tuning for the VC Enable malloc metrics for the VC Jun 20, 2022
@michaelsproul
Copy link
Member Author

I've revised this PR to be a bit more conservative after I saw the memory usage spike on the VC. However it seems that this is standard behaviour even with previous releases 🤔

vc-memory

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Jun 20, 2022
@michaelsproul
Copy link
Member Author

bors r+

bors bot pushed a commit that referenced this pull request Jun 20, 2022
## Issue Addressed

Following up from #3223 (comment), it has been observed that the validator client uses vastly more memory in some compilation configurations than others. Compiling with Cross and then putting the binary into an Ubuntu 22.04 image seems to use 3x more memory than compiling with Cargo directly on Debian bullseye.

## Proposed Changes

Enable malloc metrics for the validator client. This will hopefully allow us to see the difference between the two compilation configs and compare heap fragmentation. This PR doesn't enable malloc tuning for the VC because it was found to perform significantly worse. The `--disable-malloc-tuning` flag is repurposed to just disable the metrics.
@bors bors bot changed the title Enable malloc metrics for the VC [Merged by Bors] - Enable malloc metrics for the VC Jun 21, 2022
@bors bors bot closed this Jun 21, 2022
@michaelsproul michaelsproul deleted the vc-malloc-tuning branch June 21, 2022 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
low-hanging-fruit Easy to resolve, get it before someone else does! ready-for-merge This PR is ready to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants