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

Faster Clone() in kv/key.go #11957

Merged
merged 2 commits into from
Sep 10, 2019
Merged

Faster Clone() in kv/key.go #11957

merged 2 commits into from
Sep 10, 2019

Conversation

zhangyunhao116
Copy link
Contributor

@zhangyunhao116 zhangyunhao116 commented Aug 30, 2019

The new function use a predefined byte slice with length len(k) instead of use an undefined []byte(nil). The latter will grow in copy procession, and since Key is just an alias for []byte, so we can use len(k) instead of len([]byte(k)), the former is more readable.

@sre-bot sre-bot added the contribution This PR is from a community contributor. label Aug 30, 2019
@zhangyunhao116
Copy link
Contributor Author

A simple benchmark for this change:
BenchmarkDeepCopyBytes-12 100000000 12.8 ns/op 5 B/op 1 allocs/op
BenchmarkDeepCopyBytes2-12 100000000 18.0 ns/op 8 B/op 1 allocs/op

DeepCopyBytes is the new one, test value is []byte("11111")

@codecov
Copy link

codecov bot commented Aug 30, 2019

Codecov Report

Merging #11957 into master will decrease coverage by 0.3823%.
The diff coverage is 100%.

@@               Coverage Diff                @@
##             master     #11957        +/-   ##
================================================
- Coverage   81.6088%   81.2264%   -0.3824%     
================================================
  Files           449        443         -6     
  Lines         96916      94921      -1995     
================================================
- Hits          79092      77101      -1991     
- Misses        12252      12334        +82     
+ Partials       5572       5486        -86

@zz-jason
Copy link
Member

@ZYunH Thanks for your contributing, please follow the Commit Message and Pull Request Style to reformat the PR title.

@zz-jason zz-jason added the type/enhancement The issue or PR belongs to an enhancement. label Sep 10, 2019
Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

LGTM

@zz-jason zz-jason added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 10, 2019
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.

LGTM

@jackysp jackysp added the status/can-merge Indicates a PR has been approved by a committer. label Sep 10, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Sep 10, 2019

Your auto merge job has been accepted, waiting for #11953, #12052

@sre-bot
Copy link
Contributor

sre-bot commented Sep 10, 2019

/run-all-tests

@sre-bot sre-bot merged commit 660ce3f into pingcap:master Sep 10, 2019
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. status/can-merge Indicates a PR has been approved by a committer. 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