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

table: avoid redundant encode in insert/update/delete. #11532

Merged
merged 16 commits into from
Aug 7, 2019

Conversation

crazycs520
Copy link
Contributor

@crazycs520 crazycs520 commented Jul 30, 2019

What problem does this PR solve?

  • avoid redundant encode in AddRecord.

Benchmark AddRecord function

-- create table with 10 timestamp column.
create table t (c1 timestamp, c2 timestamp .... c10 timestamp);

-- benchmark AddRecord function. The test function was below.

-- before this pr.
AddRecord              7.8s/100w ops

-- this pr
AddRecord              6.0s/100w ops

benchmark insert

Use prepare to batch insert 50w rows to table, which table contains 60 columns, 20 int columns, 20 timestamp columns, 20 varchar(50) columns.

-- before this pr
insert 50w rows spend 42s.

-- this pr
insert 50w rows spend 45s.

Before This PR
image

This PR
image

benchmark update

update t_wide set c0=c0+1 where c0>0          and c0<20000;
update t_wide set c0=c0+1 where c0>20000 and c0<40000;
update t_wide set c0=c0+1 where c0>40000 and c0<60000;
. . .
-- before this pr
update 50w rows spend 34s.

-- this pr
update 50w rows spend 41s.

Before This PR
image

This PR
image

benchmark delete

set @@tidb_batch_delete=1;
set @@tidb_dml_batch_size=2000;
-- before this pr
delete 50w rows spend 16s.

-- this pr
delete 50w rows spend 13s.

Before This PR
image

This PR
image

What is changed and how it works?

Check List

Tests

  • No test. Tiny refactor.
  • benchmark AddRecord function code.
// Add below test in table/tables/tables_test.go and run `go test -check.f "TestAddRecordForBench"`
func (ts *testSuite) TestAddRecordForBench(c *C) {
	ctx := context.Background()
	_, err := ts.se.Execute(ctx, "drop table if exists test.t")
	c.Assert(err, IsNil)

	benchNum := 1000000
	colNum := 10
	colTp := "timestamp"
	colValue := types.Time{Time: types.FromGoTime(time.Now()), Type: mysql.TypeTimestamp, Fsp: 6}

	sql := "CREATE TABLE test.t ("
	for i := 0; i < colNum; i++ {
		if i > 0 {
			sql += ", "
		}
		sql += fmt.Sprintf("c%d %s", i, colTp)
	}
	sql += ")"

	_, err = ts.se.Execute(ctx, sql)
	c.Assert(err, IsNil)
	tb, err := ts.dom.InfoSchema().TableByName(model.NewCIStr("test"), model.NewCIStr("t"))
	c.Assert(err, IsNil)

	values := make([]interface{}, 0, colNum)
	for i := 0; i < colNum; i++ {
		values = append(values, colValue)
	}

	sctx := ts.se
	c.Assert(sctx.NewTxn(ctx), IsNil)
	txn, err := sctx.Txn(true)
	c.Assert(err, IsNil)
	start := time.Now()
	row := types.MakeDatums(values...)
	for i := 0; i < benchNum; i++ {
		if i%100000 == 0 {
			c.Assert(txn.Rollback(), IsNil)
			c.Assert(sctx.NewTxn(ctx), IsNil)
			txn, err = sctx.Txn(true)
			c.Assert(err, IsNil)
		}
		_, err = tb.AddRecord(sctx, row)
		c.Assert(err, IsNil)
	}
	fmt.Printf("\n\nbench add recored : %v\n\n", time.Since(start).Seconds())
	c.Assert(txn.Rollback(), IsNil)
}

Code changes

Side effects

Related changes

@crazycs520 crazycs520 added the type/enhancement The issue or PR belongs to an enhancement. label Jul 30, 2019
@crazycs520
Copy link
Contributor Author

/run-all-tests

@crazycs520
Copy link
Contributor Author

@eurekaka PTAL

@codecov
Copy link

codecov bot commented Jul 30, 2019

Codecov Report

Merging #11532 into master will decrease coverage by 0.5225%.
The diff coverage is 93.75%.

@@               Coverage Diff               @@
##             master    #11532        +/-   ##
===============================================
- Coverage   81.7785%   81.256%   -0.5226%     
===============================================
  Files           426       426                
  Lines         94345     92035      -2310     
===============================================
- Hits          77154     74784      -2370     
- Misses        11788     11872        +84     
+ Partials       5403      5379        -24

table/tables/tables.go Outdated Show resolved Hide resolved
tablecodec/tablecodec.go Outdated Show resolved Hide resolved
@crazycs520
Copy link
Contributor Author

@eurekaka PTAL

@crazycs520
Copy link
Contributor Author

/run-all-tests

tablecodec/tablecodec.go Outdated Show resolved Hide resolved
table/tables/tables.go Outdated Show resolved Hide resolved
@crazycs520
Copy link
Contributor Author

/run-all-tests

Copy link
Contributor

@eurekaka eurekaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

table/tables/tables.go Outdated Show resolved Hide resolved
@eurekaka eurekaka added the status/LGT1 Indicates that a PR has LGTM 1. label Aug 6, 2019
@crazycs520
Copy link
Contributor Author

/run-all-tests

@@ -201,6 +237,21 @@ func encodeSignedInt(b []byte, v int64, comparable bool) []byte {
return b
}

var varIntSize = []int64{64, 8192, 1048576, 134217728, 17179869184, 2199023255552, 281474976710656, 36028797018963968, 4611686018427387904}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to use some bit shifts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great, thanks.done.

@@ -212,6 +263,17 @@ func encodeUnsignedInt(b []byte, v uint64, comparable bool) []byte {
return b
}

var varUintSize = []uint64{128, 16384, 2097152, 268435456, 34359738368, 4398046511104, 562949953421312, 72057594037927936, 9223372036854775808}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great, thanks.done.

Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rest LGTM

@crazycs520
Copy link
Contributor Author

/run-all-tests

@crazycs520 crazycs520 changed the title table: avoid redundant encode in AddRecord table: avoid redundant encode in insert/update/delete. Aug 6, 2019
@crazycs520
Copy link
Contributor Author

/run-unit-test

@jackysp jackysp merged commit 0dc9106 into pingcap:master Aug 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/LGT1 Indicates that a PR has LGTM 1. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants