Skip to content

optimize Date format#1337

Merged
windtalker merged 12 commits intopingcap:masterfrom
windtalker:date_format_opt
Jan 7, 2021
Merged

optimize Date format#1337
windtalker merged 12 commits intopingcap:masterfrom
windtalker:date_format_opt

Conversation

@windtalker
Copy link
Contributor

@windtalker windtalker commented Jan 5, 2021 •
edited by JaySon-Huang
Loading

What problem does this PR solve?

Problem Summary:

DateFormat is slow in TiFlash, need to optimize it.

What is changed and how it works?

What's Changed:

How it Works:

  1. avoid all sprintf in MyTimeBase::dateFormat
  2. precompile format string to avoid parsing format string everytime
  3. pre-allocate && reuse string capacity in FunctionDateFormat

Result:

on TPCH-20, with batchcop

query with optimize without optimize
select count(date_format(L_SHIPDATE, '%Y-%m-%d')) from lineitem 1.26 sec 5.14 sec

Related changes

Check List

Tests

  • Integration test
  • Manual test (add detailed scripts or steps below)

Side effects

Release note

  • optimize the performance of date_format function in TiFlash

@windtalker windtalker requested a review from hanfei1991 January 5, 2021 09:00
leiysky
leiysky reviewed Jan 5, 2021
View reviewed changes
hanfei1991
hanfei1991 reviewed Jan 6, 2021
View reviewed changes
Copy link
Member

@hanfei1991 hanfei1991 left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Jan 6, 2021
@windtalker
Copy link
Contributor Author

windtalker commented Jan 6, 2021

/run-all-tests

@leiysky
Copy link
Contributor

leiysky commented Jan 6, 2021

LGTM

@ti-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Jan 6, 2021
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Jan 6, 2021
@windtalker windtalker added the status/can-merge Indicates a PR has been approved by a committer. label Jan 6, 2021
@ti-srebot
Copy link
Collaborator

ti-srebot commented Jan 6, 2021

/run-all-tests

@ti-srebot
Copy link
Collaborator

ti-srebot commented Jan 6, 2021

@windtalker merge failed.

@windtalker windtalker added needs-cherry-pick-release-4.0 PR which needs to be cherry-picked to release-4.0 needs-cherry-pick-release-5.0-rc PR which needs to be cherry-picked to release-5.0-rc labels Jan 7, 2021
@windtalker
Copy link
Contributor Author

windtalker commented Jan 7, 2021

/merge

@ti-srebot
Copy link
Collaborator

ti-srebot commented Jan 7, 2021

/run-all-tests

@ti-srebot
Copy link
Collaborator

ti-srebot commented Jan 7, 2021

@windtalker merge failed.

@windtalker
Copy link
Contributor Author

windtalker commented Jan 7, 2021

/run-all-tests

@windtalker windtalker merged commit f452ab3 into pingcap:master Jan 7, 2021
@windtalker windtalker deleted the date_format_opt branch January 7, 2021 05:32
This was referenced Jan 7, 2021
Merged
Merged
ti-srebot added a commit that referenced this pull request Jan 20, 2021
@ti-srebot
optimize Date format (#1337) (#1339)
c0e1b09
ti-srebot added a commit that referenced this pull request Jan 20, 2021
@ti-srebot
optimize Date format (#1337) (#1340)
c9c9a1a
@JaySon-Huang JaySon-Huang mentioned this pull request May 21, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ti-srebot ti-srebot ti-srebot approved these changes

+2 more reviewers

@hanfei1991 hanfei1991 hanfei1991 left review comments

@leiysky leiysky leiysky left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

needs-cherry-pick-release-4.0 PR which needs to be cherry-picked to release-4.0 needs-cherry-pick-release-5.0-rc PR which needs to be cherry-picked to release-5.0-rc status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@windtalker @leiysky @ti-srebot @hanfei1991