-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
executor: add some memory tracker in HashJoin #33918
Conversation
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
executor/hash_table.go
Outdated
@@ -273,6 +277,7 @@ type baseHashTable interface { | |||
Put(hashKey uint64, rowPtr chunk.RowPtr) | |||
Get(hashKey uint64) (rowPtrs []chunk.RowPtr) | |||
Len() uint64 | |||
GetMemoryDelta() int64 |
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.
Add a comment for this func
executor/hash_table.go
Outdated
@@ -283,6 +288,9 @@ type unsafeHashTable struct { | |||
hashMap map[uint64]*entry | |||
entryStore *entryStore | |||
length uint64 | |||
|
|||
bInMap int64 // indicate there are 2^bInMap buckets in hashMap | |||
memDelta int64 |
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.
add a comment for this
// GetMemoryDelta gets the memDelta of the concurrentMapHashTable. | ||
func (ht *unsafeHashTable) GetMemoryDelta() int64 { | ||
memDelta := ht.memDelta | ||
ht.memDelta = 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.
Why set this to 0?
executor/hash_table.go
Outdated
// concurrentMapHashTable is a concurrent hash table built on concurrentMap | ||
type concurrentMapHashTable struct { | ||
hashMap concurrentMap | ||
entryStore *entryStore | ||
length uint64 | ||
memDelta int64 |
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.
Add a comment for this
Code Coverage Details: https://codecov.io/github/pingcap/tidb/commit/57dfb7a02868431ea1613d17e0da03aa5ad99049 |
executor/hash_table.go
Outdated
@@ -357,3 +387,15 @@ func (ht *concurrentMapHashTable) Get(hashKey uint64) (rowPtrs []chunk.RowPtr) { | |||
} | |||
return | |||
} | |||
|
|||
// GetMemoryDelta gets the memDelta of the concurrentMapHashTable. Memory delta will be cleared after each fetch. | |||
func (ht *concurrentMapHashTable) GetMemoryDelta() int64 { |
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.
GetAndCleanMemoryDelta
/merge |
This pull request has been accepted and is ready to merge. Commit hash: 7d358db
|
/run-all-tests |
TiDB MergeCI notify
|
What problem does this PR solve?
Issue Number: ref #33877
close #35627
close #35628
Problem Summary:
What is changed and how it works?
Track the memory usage of Golang map and entryStore in HashJoin.
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.