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

Optimize Date/Date32 scan #1374

Merged
merged 1 commit into from
Aug 19, 2024
Merged

Conversation

ShoshinNikita
Copy link
Contributor

Summary

  • Get year, month, day via a single call to time.Time.Date and hour, minute, seconds via time.Time.Clock. This change eliminates multiple calls to time.Time.date and time.Time.abs.
  • Don't allocate new time.Time if desired location is UTC.

Benchmark results of getTimeWithDifferentLocation:

name                             old time/op    new time/op    delta
GetTimeWithDifferentLocation-32    32.8ns ± 3%    17.4ns ± 3%  -46.83%  (p=0.000 n=10+10)

name                             old alloc/op   new alloc/op   delta
GetTimeWithDifferentLocation-32     0.00B          0.00B          ~     (all equal)

name                             old allocs/op  new allocs/op  delta
GetTimeWithDifferentLocation-32      0.00           0.00          ~     (all equal)
Raw benchmark results

Old version

// "old" version
func getTimeWithDifferentLocation(t time.Time, loc *time.Location) time.Time {
	return time.Date(t.Year(), t.Month(), t.Day(), t.Hour(), t.Minute(), t.Second(), t.Nanosecond(), loc)
}
goos: linux
goarch: amd64
pkg: github.com/ClickHouse/clickhouse-go/v2/lib/column
cpu: 13th Gen Intel(R) Core(TM) i9-13900HX
BenchmarkGetTimeWithDifferentLocation
BenchmarkGetTimeWithDifferentLocation-32    	36481345	        33.37 ns/op	       0 B/op	       0 allocs/op
BenchmarkGetTimeWithDifferentLocation-32    	33599790	        32.63 ns/op	       0 B/op	       0 allocs/op
BenchmarkGetTimeWithDifferentLocation-32    	38609709	        33.81 ns/op	       0 B/op	       0 allocs/op
BenchmarkGetTimeWithDifferentLocation-32    	37585620	        32.85 ns/op	       0 B/op	       0 allocs/op
BenchmarkGetTimeWithDifferentLocation-32    	38654622	        32.43 ns/op	       0 B/op	       0 allocs/op
BenchmarkGetTimeWithDifferentLocation-32    	37217412	        33.23 ns/op	       0 B/op	       0 allocs/op
BenchmarkGetTimeWithDifferentLocation-32    	34944349	        32.15 ns/op	       0 B/op	       0 allocs/op
BenchmarkGetTimeWithDifferentLocation-32    	37236122	        32.88 ns/op	       0 B/op	       0 allocs/op
BenchmarkGetTimeWithDifferentLocation-32    	38284749	        32.53 ns/op	       0 B/op	       0 allocs/op
BenchmarkGetTimeWithDifferentLocation-32    	36064225	        32.32 ns/op	       0 B/op	       0 allocs/op
PASS
ok  	github.com/ClickHouse/clickhouse-go/v2/lib/column	12.454s

Optimized version

goos: linux
goarch: amd64
pkg: github.com/ClickHouse/clickhouse-go/v2/lib/column
cpu: 13th Gen Intel(R) Core(TM) i9-13900HX
BenchmarkGetTimeWithDifferentLocation
BenchmarkGetTimeWithDifferentLocation-32    	73176572	        17.92 ns/op	       0 B/op	       0 allocs/op
BenchmarkGetTimeWithDifferentLocation-32    	63622671	        17.43 ns/op	       0 B/op	       0 allocs/op
BenchmarkGetTimeWithDifferentLocation-32    	73126280	        17.48 ns/op	       0 B/op	       0 allocs/op
BenchmarkGetTimeWithDifferentLocation-32    	69986809	        17.41 ns/op	       0 B/op	       0 allocs/op
BenchmarkGetTimeWithDifferentLocation-32    	69220406	        17.56 ns/op	       0 B/op	       0 allocs/op
BenchmarkGetTimeWithDifferentLocation-32    	70910238	        17.39 ns/op	       0 B/op	       0 allocs/op
BenchmarkGetTimeWithDifferentLocation-32    	71086362	        17.63 ns/op	       0 B/op	       0 allocs/op
BenchmarkGetTimeWithDifferentLocation-32    	72798584	        17.16 ns/op	       0 B/op	       0 allocs/op
BenchmarkGetTimeWithDifferentLocation-32    	68707909	        17.33 ns/op	       0 B/op	       0 allocs/op
BenchmarkGetTimeWithDifferentLocation-32    	65608513	        17.19 ns/op	       0 B/op	       0 allocs/op
PASS
ok  	github.com/ClickHouse/clickhouse-go/v2/lib/column	12.387s

Checklist

Delete items not relevant to your PR:

  • Unit and integration tests covering the common scenarios were added
  • A human-readable description of the changes was provided to include in CHANGELOG

@CLAassistant
Copy link

CLAassistant commented Aug 15, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@jkaflik jkaflik left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for your contribution!

@jkaflik jkaflik merged commit dc05f5e into ClickHouse:main Aug 19, 2024
12 checks passed
TylerHelmuth referenced this pull request in open-telemetry/opentelemetry-collector-contrib Aug 20, 2024
….27.2 (#34752)

[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
|
[github.com/ClickHouse/clickhouse-go/v2](https://togithub.com/ClickHouse/clickhouse-go)
| `v2.27.1` -> `v2.27.2` |
[![age](https://developer.mend.io/api/mc/badges/age/go/github.com%2fClickHouse%2fclickhouse-go%2fv2/v2.27.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/go/github.com%2fClickHouse%2fclickhouse-go%2fv2/v2.27.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/go/github.com%2fClickHouse%2fclickhouse-go%2fv2/v2.27.1/v2.27.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/go/github.com%2fClickHouse%2fclickhouse-go%2fv2/v2.27.1/v2.27.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency
Dashboard for more information.

---

### Release Notes

<details>
<summary>ClickHouse/clickhouse-go
(github.com/ClickHouse/clickhouse-go/v2)</summary>

###
[`v2.27.2`](https://togithub.com/ClickHouse/clickhouse-go/blob/HEAD/CHANGELOG.md#v2272-2024-08-20----Release-notes-generated-using-configuration-in-githubreleaseyml-at-main---)

[Compare
Source](https://togithub.com/ClickHouse/clickhouse-go/compare/v2.27.1...v2.27.2)

#### What's Changed

##### Enhancements 🎉

- Optimize Date/Date32 scan by
[@&#8203;ShoshinNikita](https://togithub.com/ShoshinNikita) in
[https://github.com/ClickHouse/clickhouse-go/pull/1374](https://togithub.com/ClickHouse/clickhouse-go/pull/1374)

##### Fixes 🐛

- Fix column list parsing for multiline INSERT statements by
[@&#8203;Fiery-Fenix](https://togithub.com/Fiery-Fenix) in
[https://github.com/ClickHouse/clickhouse-go/pull/1373](https://togithub.com/ClickHouse/clickhouse-go/pull/1373)

#### New Contributors

- [@&#8203;Fiery-Fenix](https://togithub.com/Fiery-Fenix) made their
first contribution in
[https://github.com/ClickHouse/clickhouse-go/pull/1373](https://togithub.com/ClickHouse/clickhouse-go/pull/1373)
- [@&#8203;ShoshinNikita](https://togithub.com/ShoshinNikita) made their
first contribution in
[https://github.com/ClickHouse/clickhouse-go/pull/1374](https://togithub.com/ClickHouse/clickhouse-go/pull/1374)

**Full Changelog**:
ClickHouse/clickhouse-go@v2.27.1...v2.27.2

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "on tuesday" (UTC), Automerge - At any
time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View the
[repository job
log](https://developer.mend.io/github/open-telemetry/opentelemetry-collector-contrib).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC4yNi4xIiwidXBkYXRlZEluVmVyIjoiMzguMjYuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiZGVwZW5kZW5jaWVzIiwicmVub3ZhdGVib3QiXX0=-->

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: opentelemetrybot <[email protected]>
Co-authored-by: Yang Song <[email protected]>
@ShoshinNikita ShoshinNikita deleted the optimize-date-scan branch August 22, 2024 20:29
f7o referenced this pull request in f7o/opentelemetry-collector-contrib Sep 12, 2024
….27.2 (open-telemetry#34752)

[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
|
[github.com/ClickHouse/clickhouse-go/v2](https://togithub.com/ClickHouse/clickhouse-go)
| `v2.27.1` -> `v2.27.2` |
[![age](https://developer.mend.io/api/mc/badges/age/go/github.com%2fClickHouse%2fclickhouse-go%2fv2/v2.27.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/go/github.com%2fClickHouse%2fclickhouse-go%2fv2/v2.27.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/go/github.com%2fClickHouse%2fclickhouse-go%2fv2/v2.27.1/v2.27.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/go/github.com%2fClickHouse%2fclickhouse-go%2fv2/v2.27.1/v2.27.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency
Dashboard for more information.

---

### Release Notes

<details>
<summary>ClickHouse/clickhouse-go
(github.com/ClickHouse/clickhouse-go/v2)</summary>

###
[`v2.27.2`](https://togithub.com/ClickHouse/clickhouse-go/blob/HEAD/CHANGELOG.md#v2272-2024-08-20----Release-notes-generated-using-configuration-in-githubreleaseyml-at-main---)

[Compare
Source](https://togithub.com/ClickHouse/clickhouse-go/compare/v2.27.1...v2.27.2)

#### What's Changed

##### Enhancements 🎉

- Optimize Date/Date32 scan by
[@&open-telemetry#8203;ShoshinNikita](https://togithub.com/ShoshinNikita) in
[https://github.com/ClickHouse/clickhouse-go/pull/1374](https://togithub.com/ClickHouse/clickhouse-go/pull/1374)

##### Fixes 🐛

- Fix column list parsing for multiline INSERT statements by
[@&open-telemetry#8203;Fiery-Fenix](https://togithub.com/Fiery-Fenix) in
[https://github.com/ClickHouse/clickhouse-go/pull/1373](https://togithub.com/ClickHouse/clickhouse-go/pull/1373)

#### New Contributors

- [@&open-telemetry#8203;Fiery-Fenix](https://togithub.com/Fiery-Fenix) made their
first contribution in
[https://github.com/ClickHouse/clickhouse-go/pull/1373](https://togithub.com/ClickHouse/clickhouse-go/pull/1373)
- [@&open-telemetry#8203;ShoshinNikita](https://togithub.com/ShoshinNikita) made their
first contribution in
[https://github.com/ClickHouse/clickhouse-go/pull/1374](https://togithub.com/ClickHouse/clickhouse-go/pull/1374)

**Full Changelog**:
ClickHouse/clickhouse-go@v2.27.1...v2.27.2

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "on tuesday" (UTC), Automerge - At any
time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View the
[repository job
log](https://developer.mend.io/github/open-telemetry/opentelemetry-collector-contrib).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC4yNi4xIiwidXBkYXRlZEluVmVyIjoiMzguMjYuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiZGVwZW5kZW5jaWVzIiwicmVub3ZhdGVib3QiXX0=-->

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: opentelemetrybot <[email protected]>
Co-authored-by: Yang Song <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants