-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
[Enhancement](scanner) allocate blocks in scanner_context on demand and free them on close #19389
Conversation
run buildall |
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
clang-tidy review says "All clean, LGTM! 👍" |
TeamCity pipeline, clickbench performance test result: |
run buildall |
clang-tidy review says "All clean, LGTM! 👍" |
run p0 |
run buildall |
clang-tidy review says "All clean, LGTM! 👍" |
run p0 |
1 similar comment
run p0 |
This may have a large performance decrease. In this case, it means the block is allocated by scanner thread and used by fragment thread or released by fragment thread. In jemalloc, it will track the arena the memory is allocated from and it has to return the memory to the arena again when release. Every thread is bond to an arena. There will be lock or condition competition。 |
clang-tidy review says "All clean, LGTM! 👍" |
run buildall |
It seems that there is no performance loss in ssb flat test. I pasted the test result above. |
clang-tidy review says "All clean, LGTM! 👍" |
run buildall |
TeamCity pipeline, clickbench performance test result: |
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
…demand and free them on close (apache#19389)" This reverts commit 6efe6ef.
Proposed changes
Issue Number: close #19283
Problem summary
Firstly, to reduce memory usage, we do not pre-allocate blocks, instead we lazily allocate block when upper call
get_free_block
. And when upper callreturn_free_block
to return free block, we add the block to a queue for memory reuse, and we will free the blocks in the queue when the scanner_context was closed instead of destructed.Secondly, to limit the memory usage of the scanner, we introduce a variable
_free_blocks_capacity
to indicate the current number of free blocks available to the scanners. The number of scanners that can be scheduled will be calculated based on this value.ssb flat test
previous
after
Checklist(Required)
Further comments
If this is a relatively large or complex change, kick off the discussion at [email protected] by explaining why you chose the solution you did and what alternatives you considered, etc...