-
Notifications
You must be signed in to change notification settings - Fork 32
extend mempool #112
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
extend mempool #112
Conversation
src/config.h
Outdated
| std::string link_type; | ||
| int minimal_allocate_size; // unit: KB | ||
| int num_stream; // can only be 1,2,4, number of stream for each client | ||
| bool autoincrease; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
auto_increase?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
| ibv_mtu active_mtu; | ||
|
|
||
| bool extend_in_flight = false; | ||
| std::atomic<unsigned int> opened_ipc{0}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need a comment to indicate the define and usage of opened_ipc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
src/mempool.cpp
Outdated
|
|
||
| auto total_blocks = mempools_[i]->get_total_blocks(); | ||
| auto allocated_blocks = mempools_[i]->get_allocated_blocks(); | ||
| INFO( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might be DEBUG instead of INFO. if we have large number for mempool
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
|
|
||
| #define BLOCK_USAGE_RATIO 0.5 | ||
| #define EXTEND_POOL_SIZE 10 << 30 | ||
| #define EXTEND_BLOCK_SIZE 64 << 10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it a problem if the preallocated block size with 32k and the extend block_size as 64k ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
64K is the default minimal-allocate-size, if we consider this kind of details, it will take a lot of time... e.g., add more verification for the input parameters
infinistore/lib.py
Outdated
| prealloc_size (int): The preallocation size. Defaults to 16. | ||
| minimal_allocate_size (int): The minimal allocation size. Defaults to 64. | ||
| num_stream (int): The number of streams. Defaults to 1. | ||
| autoincrease (bool): indicate if infinistore will be automatically increased. 10GB each time. Default False. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would recommend to set a cap for the dynamically allocated RAM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this parameter is just a flag to enable auto increase or not,
thesues
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Uh oh!
There was an error while loading. Please reload this page.