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

util/chunk: preallocate the columns array in MutRow to avoid array resize #8348

Merged
merged 1 commit into from Nov 19, 2018

Conversation

kennytm
Copy link
Contributor

@kennytm kennytm commented Nov 18, 2018

What problem does this PR solve?

When checking the flamegraph of Lightning (which does lots of INSERTs) on a table with 37 columns, I found that MutRowFromDatums spends a noticeable amount of time in runtime.growslice. This means the capacity of c.columns isn't large enough and append() will need to reallocate and move the content (as of Go 1.11 the capacity grows like 0 → 2 → 4 → 8 → 16 → …).

What is changed and how it works?

Since the final length is known, we just preallocate the slice with the known capacity.

Check List

Tests

  • Manual test (add detailed scripts or steps below)
    • Patched the TiDB dependency in Lightning and verified that runtime.growslice no longer appears directly inside MutRowFromDatums in the pprof CPU profile.

Code changes

Side effects

Related changes


This change is Reviewable

@kennytm
Copy link
Contributor Author

kennytm commented Nov 19, 2018

/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

@eurekaka eurekaka added status/LGT1 Indicates that a PR has LGTM 1. status/all tests passed labels Nov 19, 2018
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 merged commit 87b4212 into pingcap:master Nov 19, 2018
@zz-jason zz-jason added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Nov 19, 2018
@kennytm kennytm deleted the mutrow-capacity branch November 19, 2018 03:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/execution SIG execution status/LGT2 Indicates that a PR has LGTM 2. type/performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants