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.

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

Memory management dev1 pr2 #2841

Merged
merged 7 commits into from
Nov 3, 2022
Merged

Memory management dev1 pr2 #2841

merged 7 commits into from
Nov 3, 2022

Conversation

glevkovich
Copy link
Contributor

@glevkovich glevkovich commented Nov 2, 2022 •
edited
Loading

[don’t auto-bump]

Problem Overview
This is the second group of commits related to memory management improvements done in the background.
Here we:

  1. Fix two more memory leaks.
  2. Convert some raw pointers to smart pointers.
  3. Revert a wrong commit that converted a raw pointer (in a module that is not the owner of the pointer) to a smart pointer. This caused double-free on replica shutdown.
  4. Add debug logs while shutting down, which help to understand which modules were destroyed.
  5. Add names to a few types of memory pools. This helps to understand replica creation, shutdown, and resource consumption. Every pool created must have a name, which is printed on the log.

Testing is done
Ran on Maestro, leaks are gone. All apollo tests pass. No particular new tests have been added since no functionality has changed.

tbierwiaczon and others added 7 commits November 2, 2022 18:41
m_stateTransfer pointer change to unique_ptr
8f62d91
@Efrat1
Fix memory leak of PrePrepareMsg
238901c
Retransmission manager change to unique_ptr
fd1fc43
Revert "Merge pull request #59 from glevkovich/stateTransfer_mem_leak…
fce037e 
…_fix2"

This reverts commit 25630fd, reversing
changes made to c48c61b.
bftengine, diagnostics: add trace logs to increase debuggability
c54109a 
To easily debug creation/distraction of some objects, we need to add
these logs.
SimpleThreadPool, ThreadPool: add name for better logs
25d9db3 
Here we add (and enforce) name to each queue, and delete the default
ctor. In addition, some few more log entries are added.
@Efrat1
Fix leak in RO Replica (#65)
33aab3c
@glevkovich glevkovich requested a review from a team as a code owner November 2, 2022 17:50
@glevkovich
Copy link
Contributor Author

don't auto-bump

Efrat1
Efrat1 approved these changes Nov 3, 2022
View reviewed changes
Copy link
Contributor

@Efrat1 Efrat1 left a comment

Choose a reason for hiding this comment

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

lgtm, my changes are in

@glevkovich glevkovich requested review from eyalrund, mdarade and a team November 3, 2022 13:29
yuliasherman
yuliasherman approved these changes Nov 3, 2022
View reviewed changes
Copy link
Contributor

@yuliasherman yuliasherman left a comment

Choose a reason for hiding this comment

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

LGTM

@glevkovich glevkovich merged commit 0e61f95 into vmware:master Nov 3, 2022
@glevkovich glevkovich deleted the memory_management_dev1_pr2 branch November 22, 2022 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yuliasherman yuliasherman yuliasherman approved these changes

@Efrat1 Efrat1 Efrat1 approved these changes

@hshomroni hshomroni Awaiting requested review from hshomroni

@titusen titusen Awaiting requested review from titusen

@arc-vmware arc-vmware Awaiting requested review from arc-vmware

@cloudnoize cloudnoize Awaiting requested review from cloudnoize

@eyalrund eyalrund Awaiting requested review from eyalrund

@mdarade mdarade Awaiting requested review from mdarade

Assignees
No one assigned
Labels
cla-not-required
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@glevkovich @Efrat1 @yuliasherman @eyalrund @vmwclabot