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

Replace MEM_DWRITE_Q pool with MEMPROXY class pool #1873

Closed
wants to merge 10 commits into from
Closed

Replace MEM_DWRITE_Q pool with MEMPROXY class pool #1873

wants to merge 10 commits into from

Conversation

yadij
Copy link
Contributor

@yadij yadij commented Jul 29, 2024 •
edited by rousskov
Loading

Replacing memory allocate and destruct with C++ new/delete
and in-class initialization.

@rousskov rousskov self-requested a review July 30, 2024 13:37
@yadij yadij added the S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box label Jul 31, 2024
rousskov
rousskov requested changes Jul 31, 2024
View reviewed changes
Copy link
Contributor

@rousskov rousskov left a comment

Choose a reason for hiding this comment

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

Thank you for removing MEM_DWRITE_Q.

src/fs_io.cc Outdated Show resolved Hide resolved
src/fs_io.cc Outdated Show resolved Hide resolved
src/fs_io.h Outdated Show resolved Hide resolved
src/fs_io.cc Outdated Show resolved Hide resolved
src/fs_io.cc Outdated Show resolved Hide resolved
@rousskov rousskov added S-waiting-for-author author action is expected (and usually required) and removed S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box labels Jul 31, 2024
@yadij yadij added S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box and removed S-waiting-for-author author action is expected (and usually required) labels Aug 7, 2024
@rousskov rousskov added S-waiting-for-author author action is expected (and usually required) and removed S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box labels Aug 9, 2024
@yadij
Maintenance: Upgrade MEM_DWRITE_Q memory pool
b7cc30a 
 ... to a MEMPROXY class pool.

Replacing memory allocate and destruct with C++ new/delete
and in-class initialization.
yadij and others added 3 commits September 24, 2024 13:03
@yadij
Reviewer required changes
50e4b3e 
NO logic change to buffers yet. Just moving
existing logic into constructor/destructor.
@yadij
Reviewer requested changes
0cd7a7c
@yadij @rousskov
Update src/fs_io.h
c5fbd57 
Co-authored-by: Alex Rousskov <[email protected]>
@yadij yadij requested a review from rousskov September 27, 2024 03:14
@yadij yadij added S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box and removed S-waiting-for-author author action is expected (and usually required) labels Sep 27, 2024
kinkie
kinkie reviewed Sep 27, 2024
View reviewed changes
src/fs_io.cc Outdated Show resolved Hide resolved
kinkie
kinkie previously approved these changes Sep 28, 2024
View reviewed changes
Copy link
Contributor

@kinkie kinkie left a comment

Choose a reason for hiding this comment

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

lgtm

rousskov
rousskov requested changes Sep 30, 2024
View reviewed changes
src/fs_io.h Outdated Show resolved Hide resolved
src/fs_io.h Outdated Show resolved Hide resolved
src/fs_io.h Outdated Show resolved Hide resolved
src/fs_io.cc Show resolved Hide resolved
@rousskov rousskov added S-waiting-for-author author action is expected (and usually required) and removed S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box labels Sep 30, 2024
@yadij
Separate capacity and length details
9cb4fbb 
Fixes regression in the write-merging case.
@yadij yadij added S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box and removed S-waiting-for-author author action is expected (and usually required) labels Oct 10, 2024
@yadij yadij requested a review from rousskov October 10, 2024 05:23
@rousskov rousskov changed the title Maintenance: Upgrade MEM_DWRITE_Q memory pool Replace MEM_DWRITE_Q pool with MEMPROXY class pool Oct 10, 2024
rousskov
rousskov requested changes Oct 10, 2024
View reviewed changes
Copy link
Contributor

@rousskov rousskov left a comment

Choose a reason for hiding this comment

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

I have adjusted PR title/description to remove Maintenance prefix because this PR contains changes that affect mgr:mem report contents/structure (in non-trivial ways that might even affect integration with external tools).

src/fs_io.h Outdated Show resolved Hide resolved
src/fs_io.cc Outdated Show resolved Hide resolved
src/fs_io.cc Show resolved Hide resolved
src/fs_io.cc Outdated Show resolved Hide resolved
@rousskov rousskov added S-waiting-for-author author action is expected (and usually required) and removed S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box labels Oct 10, 2024
yadij and others added 2 commits October 11, 2024 05:57
@yadij @rousskov
Update src/fs_io.cc
80f6294 
Co-authored-by: Alex Rousskov <[email protected]>
@yadij @rousskov
Update src/fs_io.cc
1321718 
Co-authored-by: Alex Rousskov <[email protected]>
@yadij yadij added S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box and removed S-waiting-for-author author action is expected (and usually required) labels Oct 10, 2024
@yadij yadij requested a review from rousskov October 10, 2024 17:26
rousskov
rousskov previously requested changes Oct 10, 2024
View reviewed changes
src/fs_io.h Outdated Show resolved Hide resolved
src/fs_io.cc Show resolved Hide resolved
@rousskov rousskov added S-waiting-for-author author action is expected (and usually required) and removed S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box labels Oct 10, 2024
yadij and others added 2 commits October 12, 2024 00:05
@yadij @rousskov
Update src/fs_io.cc
a15a2c3 
Co-authored-by: Alex Rousskov <[email protected]>
@yadij @rousskov
Update src/fs_io.h
82f92ce 
Co-authored-by: Alex Rousskov <[email protected]>
@yadij yadij requested a review from rousskov October 19, 2024 09:44
@yadij yadij added S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box and removed S-waiting-for-author author action is expected (and usually required) labels Oct 19, 2024
@rousskov rousskov dismissed their stale review October 20, 2024 14:38

Thank you for addressing my concerns.

@rousskov rousskov removed their request for review October 20, 2024 14:38
@rousskov rousskov added S-waiting-for-author author action is expected (and usually required) M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels and removed S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box S-waiting-for-author author action is expected (and usually required) labels Oct 20, 2024
@rousskov
Copy link
Contributor

I have cleared this PR for merging.

squid-anubis pushed a commit that referenced this pull request Oct 20, 2024
@yadij @squid-anubis
Replace MEM_DWRITE_Q pool with MEMPROXY class pool (#1873)
ed1a732 
Replacing memory allocate and destruct with C++ new/delete
and in-class initialization.
@squid-anubis squid-anubis added the M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels label Oct 20, 2024
@squid-anubis squid-anubis added M-merged https://github.com/measurement-factory/anubis#pull-request-labels and removed M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels labels Oct 20, 2024
@yadij yadij deleted the arc-mempools-1 branch October 28, 2024 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rousskov rousskov rousskov left review comments

@kinkie kinkie kinkie left review comments

Assignees
No one assigned
Labels
M-merged https://github.com/measurement-factory/anubis#pull-request-labels
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@yadij @rousskov @kinkie @squid-anubis