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: increase column's nullBitmap's capacity (#12470) #12981

Conversation

H-ZeX
Copy link
Contributor

@H-ZeX H-ZeX commented Oct 28, 2019

cherry-pick #12470 to release-3.1


Signed-off-by: H-ZeX hzx20112012@gmail.com

What problem does this PR solve?

nullBitmap's cap

The origin nullBitmap's capacity is a little small.
When the cap param is (x<<3)+y(0<y<8), the nullBitMap's len is only x, so the col that it can represent is (x<<3), which is small that the column number (x<<3)+y, then this slice need to be expanded.
For example, if there is 8+7=15 col, we need 2 bytes nullBitmap to represent it. But, 15>>3 only get 1, so this slice need to be expanded to 2 bytes.
If the slice expands, the memory usage increased is not little, so I think adding one byte to nullBitmap is better and necessary.
In util/chunk/column.go, there is many patterns like (x+7)>>3 such as

func (c *Column) appendMultiSameNullBitmap(notNull bool, num int) {
	numNewBytes := ((c.length + num + 7) >> 3) - len(c.nullBitmap)
sizeNulls := (n + 7) >> 3

What is changed and how it works?

  • x>>3 -> ((x+7)>>3)

Check List

Tests

existing unit test

Side effects

No.

Related changes

No.

Release note

TODO

Signed-off-by: H-ZeX <hzx20112012@gmail.com>
Copy link
Contributor

@zimulala zimulala left a comment

Choose a reason for hiding this comment

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

LGTM

@zimulala zimulala added the status/LGT1 Indicates that a PR has LGTM 1. label Oct 30, 2019
Copy link
Contributor

@lonng lonng left a comment

Choose a reason for hiding this comment

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

LGTM

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

sre-bot commented Nov 1, 2019

/run-all-tests

@sre-bot sre-bot merged commit 4a6c705 into pingcap:release-2.1 Nov 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/util status/can-merge Indicates a PR has been approved by a committer. status/LGT1 Indicates that a PR has LGTM 1. type/bugfix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants