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

feat(server): active memory fragmentation - high level flow #509

Merged
merged 2 commits into from
Nov 30, 2022
Merged

feat(server): active memory fragmentation - high level flow #509

merged 2 commits into from
Nov 30, 2022

Conversation

boazsade
Copy link
Contributor

@boazsade boazsade commented Nov 22, 2022 •
edited
Loading

Signed-off-by: Boaz Sade boaz@dragonflydb.io

This PR is a merge-able version for the high level flow of active deframation.
This contain:

  1. mi malloc support for a function that identify that underutilised pages based on memory addresses.
  2. Tests from dragonfly side for this.
  3. New defrag task in engine shard to run based on current memory usage and trigger the defrag action.
    This do not contain:
  4. Actual "low level" implementation for the defrag operation that would later add to compact object.
  5. Unit test that would verify that the task really executed the defrag operation.

romange
romange reviewed Nov 22, 2022
View reviewed changes
Copy link
Collaborator

@romange romange left a comment

Choose a reason for hiding this comment

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

what about mimalloc option that we need to enforce decommit immediately?

@boazsade boazsade force-pushed the active-mem-derag branch 2 times, most recently from b6a788e to 59597f3 Compare November 22, 2022 09:03
@romange
Copy link
Collaborator

romange commented Nov 23, 2022

please mark it ready for review when you finish applying changes so that I won't review wip.

dranikpg
dranikpg reviewed Nov 23, 2022
View reviewed changes
src/server/engine_shard_set.h Outdated Show resolved Hide resolved
src/server/engine_shard_set.cc Show resolved Hide resolved
src/server/engine_shard_set.cc Outdated Show resolved Hide resolved
src/server/engine_shard_set.cc Outdated Show resolved Hide resolved
@boazsade boazsade force-pushed the active-mem-derag branch 2 times, most recently from 6ec1ca1 to e901cef Compare November 24, 2022 12:31
@boazsade boazsade marked this pull request as ready for review November 24, 2022 13:17
@boazsade boazsade requested a review from dranikpg November 24, 2022 13:18
@boazsade boazsade force-pushed the active-mem-derag branch 2 times, most recently from 7c5ea89 to 85118d5 Compare November 27, 2022 12:17
romange
romange reviewed Nov 27, 2022
View reviewed changes
src/core/compact_object.h Outdated Show resolved Hide resolved
@boazsade boazsade marked this pull request as draft November 28, 2022 09:41
dranikpg
dranikpg reviewed Nov 28, 2022
View reviewed changes
src/server/engine_shard_set.cc Show resolved Hide resolved
src/server/engine_shard_set.cc Outdated Show resolved Hide resolved
src/server/dragonfly_test.cc Outdated Show resolved Hide resolved
src/server/engine_shard_set.cc Show resolved Hide resolved
@boazsade boazsade marked this pull request as ready for review November 28, 2022 14:07
@boazsade boazsade requested a review from dranikpg November 28, 2022 14:07
@boazsade boazsade force-pushed the active-mem-derag branch 2 times, most recently from 3eb6035 to b1157ff Compare November 29, 2022 07:11
@boazsade boazsade changed the title feat(server): active memory fragmentation #448 feat(server): active memory fragmentation - high level flow Nov 29, 2022
@boazsade boazsade force-pushed the active-mem-derag branch 2 times, most recently from 9b96e21 to 786cb25 Compare November 29, 2022 08:57
@boazsade boazsade requested a review from romange November 29, 2022 10:15
@boazsade boazsade force-pushed the active-mem-derag branch 3 times, most recently from 03062ca to 14a6ba8 Compare November 29, 2022 16:36
romange
romange reviewed Nov 29, 2022
View reviewed changes
src/core/compact_object_test.cc Outdated
return ptrs;
}

bool FindUnderutilizedMemory(const std::vector<void*>& ptrs, float ratio) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

FindUnderutilizedMemory is a wrong name.
HasUnderutilizedMemory ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will change this name

romange
romange reviewed Nov 29, 2022
View reviewed changes
src/core/compact_object_test.cc Outdated
return it != ptrs.end();
}

void DecimateMemory(std::vector<void*>* ptrs, size_t steps) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

i am not sure I understand the meaning of Decimate here. What you are trying to say?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will change the name of the function to something with better naming

@boazsade
feat(server): acrtive memory defrag high level flow
9fefb10 
Signed-off-by: Boaz Sade <boaz@dragonflydb.io>
@romange romange merged commit a379b5d into dragonflydb:main Nov 30, 2022
@boazsade boazsade mentioned this pull request Dec 1, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@romange romange romange approved these changes

@dranikpg dranikpg Awaiting requested review from dranikpg

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@boazsade @romange @dranikpg