executor: load data/batch insert improvement reducing memory allocation/free #11284

cfzjywxk · 2019-07-16T16:36:41Z

What problem does this PR solve?

reducing the memory allocation/free and copy times during batch insert/load data execution process, trying to improve the performance
refactor some code logic

plz help taking a look for row buffers if they are still used after one batch commitment, this pr will make these memories reused after one batch transaction committing, by now unit tests show good.

What is changed and how it works?

do not alloc mem for every row per batch, use one mem buf for all batches

Check List

Tests

Unit test

Code changes

Has exported function/method change
Has exported variable/fields change

Side effects

Possible performance regression
Increased code complexity

jackysp · 2019-07-17T02:03:59Z

Could you provide some memory usage test results or performance test results?

jackysp · 2019-07-17T05:00:00Z

/run-all-tests

jackysp · 2019-07-17T05:35:19Z

I think it should work, we can try it on the IDC server.

lysu · 2019-07-17T05:55:47Z

@jackysp @lysu any suggestions ?
eh.. not as expected ..
old implementation will "make as much as input rows mem buf" and for each row there will be a copy between "filled row and row buf", but the results seems not differ too much.

maybe Golang did a real good mem cache job ...

Golang using managed memory, so view rss in os level maybe useless - -, maybe can take look go's memstat https://blog.xiaoba.me/2017/09/02/how-to-play-golang-with-gdb-and-python.html.. but IMHO it's hard to observe too - - maybe take look other metric like import speed? :D

cfzjywxk · 2019-07-17T06:09:02Z

500w data load this is tested on idc machine， speed and process mem use almost the same(not very stable, sometimes even slower)

lysu · 2019-07-17T06:27:43Z

executor/load_data.go

+	if uint64(cap(e.rows)) < limit {
+		e.rows = make([][]types.Datum, 0, limit)
+		for i := 0; uint64(i) < limit; i++ {
+			e.rows = append(e.rows, make([]types.Datum, len(e.Table.Cols())))


maybe we only need new limit - cap(e.rows) types.Datum here if the limit can be dynamical change

and the other optimize maybe or not work is make([]types.Datum, (limit -cap(e.rows)) * len(e.Table.Cols())) once and then cut slice from this big array to append

seems trace not support load data now ?

@lysu currently init will be called only once. in another pr making variable settable will do this

ngaut · 2019-07-17T07:41:41Z

Please squash your commit message before you file the PR.

cfzjywxk · 2019-07-18T02:45:43Z

@jackysp PTAL

jackysp · 2019-07-18T03:25:31Z

On IDC, we can use ansible to start tikv as the storage engine. You could take a look at the tidb heap separately during the test. @lysu will help.

cfzjywxk · 2019-07-18T11:58:28Z

some tests results on idc tidb cluster, loading 500w rows sysbench table schema


master branch

Git Commit Hash: fcc15b1256e4e33132a80e39b841a6e062c353a0

mysql> load data local infile '/data3/rwork/tests/data/500w.dat' into table 2_master;
Query OK, 5000000 rows affected (13 min 39.47 sec)
Records: 5000000 Deleted: 0 Skipped: 0 Warnings: 0

Type: alloc_space
Time: Jul 18, 2019 at 7:34pm (CST)
Showing nodes accounting for 101682.08MB, 96.52% of 105347.99MB total
Dropped 775 nodes (cum <= 526.74MB)
flat flat% sum% cum cum%
42212.87MB 40.07% 40.07% 42212.87MB 40.07% github.com/pingcap/tidb/executor.(*LoadDataInfo).getLine
21196.27MB 20.12% 60.19% 21196.27MB 20.12% github.com/pingcap/goleveldb/leveldb/memdb.New
10225.16MB 9.71% 69.90% 10225.16MB 9.71% github.com/pingcap/goleveldb/leveldb/memdb.(*DB).Put
3374.63MB 3.20% 73.10% 3374.63MB 3.20% github.com/pingcap/tidb/store/tikv.(*twoPhaseCommitter).initKeysAndMutations.func1
2650.29MB 2.52% 75.62% 2650.29MB 2.52% github.com/pingcap/tidb/store/tikv.(*RegionCache).GroupKeysByRegion


dev branch(master commit position with  dev commits)

Git Commit Hash: 7f6f89c80f39a5a6e0022fd79a9b9af85fadfff9

mysql> load data local infile '/data3/rwork/tests/data/500w.dat' into table p4348_2;
Query OK, 5000000 rows affected (10 min 42.53 sec)
Records: 5000000 Deleted: 0 Skipped: 0 Warnings: 0

 

Build ID: b6b3aac95012ab192d0a3e1110aebf241dd40a9f
Type: alloc_space
Time: Jul 18, 2019 at 7:52pm (CST)
Showing nodes accounting for 101016.81MB, 96.95% of 104191.50MB total

41964.72MB 40.28% 40.28% 41964.72MB 40.28% github.com/pingcap/tidb/executor.(*LoadDataInfo).getLine
21250.88MB 20.40% 60.67% 21250.88MB 20.40% github.com/pingcap/goleveldb/leveldb/memdb.New
10198.45MB 9.79% 70.46% 10198.45MB 9.79% github.com/pingcap/goleveldb/leveldb/memdb.(*DB).Put
3477.13MB 3.34% 73.80% 3477.13MB 3.34% github.com/pingcap/tidb/store/tikv.(*twoPhaseCommitter).initKeysAndMutations.func1
2684.87MB 2.58% 76.37% 2685.37MB 2.58% github.com/pingcap/tidb/store/tikv.(*RegionCache).GroupKeysByRegion
2312.38MB 2.22% 78.59% 2312.38MB 2.22% github.com/pingcap/tidb/util/codec.reallocBytes
1815.13MB 1.74% 80.34% 1815.13MB 1.74% github.com/pingcap/tidb/executor.(*fieldWriter).outputField

ngaut · 2019-07-18T12:09:28Z

According to the memory profile output, there is not much difference between master and current PR.
It's wired, since the loading time reduce 25%.

cfzjywxk · 2019-07-18T13:13:43Z

thanks @lysu for another mem usage opt suggestion for "getLine" function,
rerun with update,

memory usage better:


dev branch(master commit position with  dev commits)

 Git Commit Hash: 7ad46a5702eb08288f7558d27ab478bc44d51bb7

mysql> load data local infile '/data3/rwork/tests/data/500w.dat' into table newt;
Query OK, 5000000 rows affected (2 min 45.06 sec)
Records: 5000000 Deleted: 0 Skipped: 0 Warnings: 0

 

start memory profile  

Showing nodes accounting for 99.66MB, 98.03% of 101.66MB total

 

end memory profile

Showing nodes accounting for 57741.90MB, 96.02% of 60136.50MB total
Dropped 553 nodes (cum <= 300.68MB)
flat flat% sum% cum cum%
20618.74MB 34.29% 34.29% 20618.74MB 34.29% github.com/pingcap/goleveldb/leveldb/memdb.New
9918.86MB 16.49% 50.78% 9918.86MB 16.49% github.com/pingcap/goleveldb/leveldb/memdb.(*DB).Put
3340.36MB 5.55% 56.34% 3340.36MB 5.55% github.com/pingcap/tidb/store/tikv.(*twoPhaseCommitter).initKeysAndMutations.func1


 

master branch

Git Commit Hash: fcc15b1256e4e33132a80e39b841a6e062c353a0

 

mysql> load data local infile '/data3/rwork/tests/data/500w.dat' into table mastert;
Query OK, 5000000 rows affected (3 min 7.95 sec)
Records: 5000000 Deleted: 0 Skipped: 0 Warnings: 0

 

start memory profile  

Showing nodes accounting for 99.61MB, 100% of 99.61MB total

end memory profile

Showing nodes accounting for 100685.37MB, 97.31% of 103467.93MB total
Dropped 489 nodes (cum <= 517.34MB)
flat flat% sum% cum cum%
41421.47MB 40.03% 40.03% 41421.47MB 40.03% github.com/pingcap/tidb/executor.(*LoadDataInfo).getLine
20895.71MB 20.20% 60.23% 20895.71MB 20.20% github.com/pingcap/goleveldb/leveldb/memdb.New
10134.26MB 9.79% 70.02% 10134.26MB 9.79% github.com/pingcap/goleveldb/leveldb/memdb.(*DB).Put

cfzjywxk · 2019-07-22T05:57:27Z

@jackysp @lysu should we merge this first and leave other optimizations in other prs so that this will not be too big diff ?

jackysp · 2019-07-22T06:34:08Z

executor/load_data.go

@@ -16,6 +16,7 @@ package executor
 import (
 	"context"
 	"fmt"
+	"github.com/pingcap/tidb/util/hack"


Please move it to the 3rd party libs.

jackysp · 2019-07-22T06:39:33Z

executor/builder.go

@@ -681,6 +681,9 @@ func (b *executorBuilder) buildLoadData(v *plannercore.LoadData) Executor {
 		},
 	}

+	var defaultLoadDataBatchCnt uint64 = 20000 //TODO this will be changed to variable in another pr


Comment should start with whitespace.

jackysp

rest LGTM

…ing copy times fix getLine function causing mem copy problem

cfzjywxk · 2019-07-22T07:02:10Z

/run-all-tests

cfzjywxk · 2019-07-22T07:24:07Z

/run-all-tests

jackysp

LGTM

jackysp · 2019-07-22T07:28:59Z

PTAL @lysu

cfzjywxk · 2019-07-22T07:35:28Z

/run-circleci-tests

codecov · 2019-07-22T07:50:15Z

Codecov Report

Merging #11284 into master will decrease coverage by 0.5427%.
The diff coverage is 92.5925%.

@@               Coverage Diff                @@
##             master     #11284        +/-   ##
================================================
- Coverage   81.8226%   81.2799%   -0.5428%     
================================================
  Files           424        423         -1     
  Lines         92131      90256      -1875     
================================================
- Hits          75384      73360      -2024     
- Misses        11480      11600       +120     
- Partials       5267       5296        +29

codecov · 2019-07-22T07:50:16Z

Codecov Report

Merging #11284 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #11284   +/-   ##
===========================================
  Coverage   81.6377%   81.6377%           
===========================================
  Files           423        423           
  Lines         91318      91318           
===========================================
  Hits          74550      74550           
  Misses        11467      11467           
  Partials       5301       5301

jackysp

LGTM

lysu

LGTM

cfzjywxk requested review from lysu, tiancaiamao, jackysp and winkyao July 16, 2019 16:36

lysu added component/server type/performance labels Jul 17, 2019

jackysp added the status/all tests passed label Jul 17, 2019

lysu reviewed Jul 17, 2019

View reviewed changes

cfzjywxk force-pushed the p4348 branch from 7f6f89c to 7ad46a5 Compare July 18, 2019 12:39

jackysp reviewed Jul 22, 2019

View reviewed changes

optimize load data/batch insert memory usage, alloc/free times, reduc…

3ea1f68

…ing copy times fix getLine function causing mem copy problem

cfzjywxk force-pushed the p4348 branch from 7ad46a5 to 3ea1f68 Compare July 22, 2019 07:00

jackysp reviewed Jul 22, 2019

View reviewed changes

lysu approved these changes Jul 23, 2019

View reviewed changes

Merge branch 'master' into p4348

67a3b77

zz-jason merged commit 119d532 into pingcap:master Jul 23, 2019

zz-jason added the status/LGT2 Indicates that a PR has LGTM 2. label Jul 23, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

executor: load data/batch insert improvement reducing memory allocation/free #11284

executor: load data/batch insert improvement reducing memory allocation/free #11284

cfzjywxk commented Jul 16, 2019

jackysp commented Jul 17, 2019

jackysp commented Jul 17, 2019

jackysp commented Jul 17, 2019

lysu commented Jul 17, 2019 •

edited

Loading

cfzjywxk commented Jul 17, 2019

lysu Jul 17, 2019

cfzjywxk Jul 17, 2019

cfzjywxk Jul 22, 2019

ngaut commented Jul 17, 2019

cfzjywxk commented Jul 18, 2019

jackysp commented Jul 18, 2019

cfzjywxk commented Jul 18, 2019

ngaut commented Jul 18, 2019

cfzjywxk commented Jul 18, 2019 •

edited

Loading

cfzjywxk commented Jul 22, 2019

jackysp Jul 22, 2019

jackysp Jul 22, 2019

jackysp left a comment

cfzjywxk commented Jul 22, 2019

cfzjywxk commented Jul 22, 2019

jackysp left a comment

jackysp commented Jul 22, 2019

cfzjywxk commented Jul 22, 2019

codecov bot commented Jul 22, 2019

codecov bot commented Jul 22, 2019 •

edited

Loading

jackysp left a comment

lysu left a comment

executor: load data/batch insert improvement reducing memory allocation/free #11284

executor: load data/batch insert improvement reducing memory allocation/free #11284

Conversation

cfzjywxk commented Jul 16, 2019

What problem does this PR solve?

What is changed and how it works?

Check List

jackysp commented Jul 17, 2019

jackysp commented Jul 17, 2019

jackysp commented Jul 17, 2019

lysu commented Jul 17, 2019 • edited Loading

cfzjywxk commented Jul 17, 2019

lysu Jul 17, 2019

Choose a reason for hiding this comment

cfzjywxk Jul 17, 2019

Choose a reason for hiding this comment

cfzjywxk Jul 22, 2019

Choose a reason for hiding this comment

ngaut commented Jul 17, 2019

cfzjywxk commented Jul 18, 2019

jackysp commented Jul 18, 2019

cfzjywxk commented Jul 18, 2019

ngaut commented Jul 18, 2019

cfzjywxk commented Jul 18, 2019 • edited Loading

cfzjywxk commented Jul 22, 2019

jackysp Jul 22, 2019

Choose a reason for hiding this comment

jackysp Jul 22, 2019

Choose a reason for hiding this comment

jackysp left a comment

Choose a reason for hiding this comment

cfzjywxk commented Jul 22, 2019

cfzjywxk commented Jul 22, 2019

jackysp left a comment

Choose a reason for hiding this comment

jackysp commented Jul 22, 2019

cfzjywxk commented Jul 22, 2019

codecov bot commented Jul 22, 2019

Codecov Report

codecov bot commented Jul 22, 2019 • edited Loading

Codecov Report

jackysp left a comment

Choose a reason for hiding this comment

lysu left a comment

Choose a reason for hiding this comment

lysu commented Jul 17, 2019 •

edited

Loading

cfzjywxk commented Jul 18, 2019 •

edited

Loading

codecov bot commented Jul 22, 2019 •

edited

Loading