Skip to content

sql: remove pointless Clock update closure allocation#57207

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
jordanlewis:clockfn
Dec 1, 2020
Merged

sql: remove pointless Clock update closure allocation#57207
craig[bot] merged 1 commit intocockroachdb:masterfrom
jordanlewis:clockfn

Conversation

@jordanlewis
Copy link
Copy Markdown
Member

Previously, we made a pointless closure on every query to update the
hlc.Clock on receipt of new observed timestamps. This is pointless
because we can be passing pre-allocated objects that can update
themselves (most often the *hlc.Clock itself).

name                                          old time/op    new time/op    delta
FlowSetup/vectorize=true/distribute=false-24     132µs ± 2%     132µs ± 1%    ~     (p=0.661 n=10+9)

name                                          old alloc/op   new alloc/op   delta
FlowSetup/vectorize=true/distribute=false-24    28.3kB ± 1%    28.2kB ± 1%    ~     (p=0.605 n=9+9)

name                                          old allocs/op  new allocs/op  delta
FlowSetup/vectorize=true/distribute=false-24       232 ± 0%       230 ± 0%  -0.86%  (p=0.000 n=8+8)

Release note: None

@jordanlewis jordanlewis requested review from a team and miretskiy and removed request for a team November 30, 2020 02:05
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@yuzefovich yuzefovich removed the request for review from miretskiy November 30, 2020 06:03
Copy link
Copy Markdown
Member

@yuzefovich yuzefovich left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 15 of 15 files at r1.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @jordanlewis)


pkg/sql/distsql_running.go, line 544 at r1 (raw file):

// timestamp. Usually wraps an hlc.Clock.
type clockUpdater interface {
	// Update updates this ClockUpdater with the the observed hlc.Timestamp.

nit: s/the the/the/.

Previously, we made a pointless closure on every query to update the
hlc.Clock on receipt of new observed timestamps. This is pointless
because we can be passing pre-allocated objects that can update
themselves (most often the *hlc.Clock itself).

Release note: None
@jordanlewis
Copy link
Copy Markdown
Member Author

tftr!

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Dec 1, 2020

Build succeeded:

@craig craig bot merged commit 22906f7 into cockroachdb:master Dec 1, 2020
@jordanlewis jordanlewis deleted the clockfn branch December 1, 2020 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants