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.

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: Reduce the memory usage of InsertExec #14568

Closed
wants to merge 2 commits into from

Conversation

b41sh
Copy link
Member

@b41sh b41sh commented Jan 22, 2020

What problem does this PR solve?

InsertExec consumes too much memory 14355

What is changed and how it works?

allocate less rows and batch exec.
we can see memory usage of insert 500,000 rows reduce from 2.3GB to 1.7GB
image

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)

@sre-bot sre-bot added the contribution This PR is from a community contributor. label Jan 22, 2020
@shenli
Copy link
Member

shenli commented Jan 22, 2020

@b41sh Thanks for your amazing work!

@jackysp
Copy link
Member

jackysp commented Jan 23, 2020

Could you show the difference of go pprof heap? @b41sh

@b41sh
Copy link
Member Author

b41sh commented Jan 23, 2020

image
image

alloc_space of insertRows drop from 11.45MB to 0.77MB
@jackysp PTAL

@jackysp
Copy link
Member

jackysp commented Jan 24, 2020

Thank, @b41sh ! But it doesn't seem to be a great improvement. Can you analyze the reason? Because this modification may bring some negative effects, such as making the insert ignore / on duplicate update statement call the BatchGet interface more times. We needs to better balance pros and cons.

@qw4990 qw4990 self-requested a review February 3, 2020 08:21
@b41sh
Copy link
Member Author

b41sh commented Feb 12, 2020

@jackysp @XuHuaiyu @qw4990 thanks for your guidance.
As the main use of memory is in e.Lists, and rows reset does not immediately reclaim memory. This PR will not bring improvement, so I close it.

@b41sh b41sh closed this Feb 12, 2020
@XuHuaiyu
Copy link
Contributor

XuHuaiyu commented Feb 12, 2020

  • It's my mistake here. I thought the most of the memory space is consumed during the execution phase, but actually:
    • 79.56%: parse phase and planBuilder phases
    • 8.34%: transaction buffer, and mutation during 2pc

屏幕快照 2020-02-12 下午5 42 07

  • I bench the heap of branch master and this commit, this commit does not bring an actual reduction of the inuse_space.

branch master

(pprof) list execute
Total: 771.81MB
ROUTINE ======================== github.com/pingcap/tidb/session.(*session).execute in /home/tidb/xhy/GOPATH/src/github.com/pingcap/tidb/session/session.go
         0   659.09MB (flat, cum) 85.39% of Total

this commit

(pprof) list execute
Total: 938.31MB
ROUTINE ======================== github.com/pingcap/tidb/session.(*session).execute in /home/tidb/xhy/GOPATH/src/github.com/pingcap/tidb/session/session.go
         0   825.72MB (flat, cum) 88.00% of Total

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution This PR is from a community contributor. type/performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants