Skip to content
This repository has been archived by the owner on Dec 8, 2021. It is now read-only.

restore: reuse bytes.Buffer #107

Merged
merged 1 commit into from
Dec 21, 2018
Merged

restore: reuse bytes.Buffer #107

merged 1 commit into from
Dec 21, 2018

Conversation

lonng
Copy link
Contributor

@lonng lonng commented Dec 21, 2018

What problem does this PR solve?

restore: reuse bytes.Buffer in chunk.restore

benchmark (SQL file: 1529669bytes ~1.5KB, Chunk size: 64KB, Rows: 223):

func BenchmarkChunkRestoreReuseBuffer(b *testing.B) {

pkg: github.com/pingcap/tidb-lightning/lightning/restore
BenchmarkChunkRestoreReuseBuffer-8           500           2506818 ns/op         8996015 B/op        816 allocs/op
BenchmarkChunkRestoreReuseBuffer2-8         1000           2100410 ns/op         5665507 B/op        630 allocs/op
PASS

benchmark (SQL file: 1GB, Chunk size: 64KB):

goos: darwin
goarch: amd64
pkg: github.com/pingcap/tidb-lightning/lightning/restore
BenchmarkChunkRestoreReuseBuffer-8             1        2829461699 ns/op        6504717832 B/op  1342278 allocs/op
BenchmarkChunkRestoreReuseBuffer2-8            1        1476907662 ns/op        3284208384 B/op  1114627 allocs/op
PASS

What is changed and how it works?

Check List

Tests

  • Unit test
  • Integration test

Code changes

Side effects

Related changes

@sre-bot
Copy link

sre-bot commented Dec 21, 2018

Hi contributor, thanks for your PR.

This patch needs to be approved by someone of admins. They should reply with "/ok-to-test" to accept this PR for running test automatically.

@lonng lonng requested a review from kennytm December 21, 2018 02:23
@lonng lonng added status/PTAL This PR is ready for review. Add this label back after committing new changes priority/unimportant type/enhancement Performance improvement or refactoring labels Dec 21, 2018
@lonng
Copy link
Contributor Author

lonng commented Dec 21, 2018

/run-all-tests

@lonng
Copy link
Contributor Author

lonng commented Dec 21, 2018

/run-integration-test

Copy link
Collaborator

@kennytm kennytm left a comment

Choose a reason for hiding this comment

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

LGTM.

PTAL @csuzhangxc

@IANTHEREAL
Copy link
Collaborator

LGTM

@IANTHEREAL IANTHEREAL added status/LGT2 Two reviewers already commented LGTM, ready for merge (LGTM2) and removed status/PTAL This PR is ready for review. Add this label back after committing new changes labels Dec 21, 2018
Copy link

@winkyao winkyao left a comment

Choose a reason for hiding this comment

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

LGTM

@kennytm kennytm merged commit c807b62 into master Dec 21, 2018
@kennytm kennytm deleted the lonng/pr-mem-reuse branch December 21, 2018 03:54
@sre-bot
Copy link

sre-bot commented Dec 21, 2018

Hi contributor, thanks for your PR.

This patch needs to be approved by someone of admins. They should reply with "/ok-to-test" to accept this PR for running test automatically.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status/LGT2 Two reviewers already commented LGTM, ready for merge (LGTM2) type/enhancement Performance improvement or refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants