Skip to content

expression: Reduce memory allocation in builtinCastTimeAsDecimalSig#12439

Merged
sre-bot merged 7 commits intopingcap:masterfrom
polyrabbit:reduce-memory-allocation
Sep 27, 2019
Merged

expression: Reduce memory allocation in builtinCastTimeAsDecimalSig#12439
sre-bot merged 7 commits intopingcap:masterfrom
polyrabbit:reduce-memory-allocation

Conversation

@polyrabbit
Copy link
Contributor

What problem does this PR solve?

This PR reduces memory allocation, according to @SunRunAway 's comments in #12426

What is changed and how it works?

This change reduces ~10% memory allocation:

$ go test -v -benchmem -bench=BenchmarkVectorizedBuiltinCastFunc -run=BenchmarkVectorizedBuiltinCastFunc
goos: darwin
goarch: amd64
pkg: github.com/pingcap/tidb/expression
...
BenchmarkVectorizedBuiltinCastFunc/builtinCastTimeAsDecimalSig-VecBuiltinFunc-8             	    3000	    490582 ns/op	  116513 B/op	    8023 allocs/op
BenchmarkVectorizedBuiltinCastFunc/builtinCastTimeAsDecimalSig-NonVecBuiltinFunc-8          	    3000	    529352 ns/op	  155393 B/op	    8833 allocs/op

Check List

Tests

  • Unit test

Code changes

  • Has exported function/method change

@codecov
Copy link

codecov bot commented Sep 27, 2019

Codecov Report

Merging #12439 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #12439   +/-   ##
===========================================
  Coverage   79.7515%   79.7515%           
===========================================
  Files           462        462           
  Lines        102304     102304           
===========================================
  Hits          81589      81589           
  Misses        14828      14828           
  Partials       5887       5887

@SunRunAway SunRunAway changed the title expression: Reduce memory allocation expression: Reduce memory allocation in builtinCastTimeAsDecimalSig Sep 27, 2019
Copy link
Contributor

@SunRunAway SunRunAway left a comment

Choose a reason for hiding this comment

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

LGTM

@SunRunAway SunRunAway requested a review from qw4990 September 27, 2019 09:03
@polyrabbit
Copy link
Contributor Author

/run-unit-test

Copy link
Contributor

@qw4990 qw4990 left a comment

Choose a reason for hiding this comment

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

LGTM

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

sre-bot commented Sep 27, 2019

/run-all-tests

@sre-bot sre-bot merged commit aaf1178 into pingcap:master Sep 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/expression contribution This PR is from a community contributor. status/can-merge Indicates a PR has been approved by a committer.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants