planner: Expression memory trace #37624
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. |
ti-chi-bot
added
release-note-none
…tidb into expression_memory_trace
ti-chi-bot
removed
the
do-not-merge/work-in-progress
expression/scalar_function.go
Outdated
| if sf.Function == nil { | ||
| sum += sf.Function.MemoryUsage() | ||
| } |
There was a problem hiding this comment.
Need we to handle this when the sf.Function != nil?
There was a problem hiding this comment.
Need we to handle this when the
sf.Function != nil?
You are right, we have handle nil in the inner MemoryUsage()
expression/builtin.go
Outdated
| } | ||
|
|
||
| sum = emptyBaseBuiltinFunc + b.bufAllocator.MemoryUsage() + | ||
| b.tp.MemoryUsage() + int64(unsafe.Sizeof(*b.childrenVectorizedOnce)) + |
There was a problem hiding this comment.
Can childrenVectorizedOnce and childrenReversedOnce be nil?
There was a problem hiding this comment.
I suggest to define a const number for Once like const onceSize = int64(unsafe.Sizeof(Once{})) instead of copying it here.
expression/column.go
Outdated
| return | ||
| } | ||
|
|
||
| sum = col.Column.MemoryUsage() + int64(unsafe.Sizeof(col.Data)) |
There was a problem hiding this comment.
Better to implement MemoryUsage for Datum since it is commonly used in TiDB.
expression/column.go
Outdated
| return | ||
| } | ||
|
|
||
| sum = emptyColumnSize + col.RetType.MemoryUsage() + int64(cap(col.hashcode))*int64(unsafe.Sizeof(*new(byte))) + |
There was a problem hiding this comment.
Is int64(cap(col.hashcode)) enough? If I remember correctly, the size of 1 byte is always 1?
expression/column_test.go
Outdated
| @@ -263,3 +263,13 @@ func TestGcColumnExprIsTidbShard(t *testing.T) { | |||
| shardExpr := NewFunctionInternal(ctx, ast.TiDBShard, ft, col) | |||
| require.True(t, GcColumnExprIsTidbShard(shardExpr)) | |||
| } | |||
|
|
|||
| func TestColumnMemoryUsage(t *testing.T) { | |||
There was a problem hiding this comment.
How about combining all these tests as one like TestExpressionMemoryUsage?
ti-chi-bot
added
status/LGT2
|
/merge |
|
This pull request has been accepted and is ready to merge. Commit hash: 316b2d3
|
ti-chi-bot
added
the
status/can-merge
TiDB MergeCI notify
|
What problem does this PR solve?
Get the memory usage of the Expression.
Issue Number: ref #37632
Problem Summary:
What is changed and how it works?
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.