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

invariants: fix non-inlined empty function call #4209

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

RaduBerinde
Copy link
Member

@RaduBerinde RaduBerinde commented Dec 22, 2024

invariants: fix non-inlined empty function call

The invariants.Sometimes() was not being inlined even though it
resolves to a return false when invariants aren't on. It looks like
the inline decision happens before some phases of dead code
elimination. This function is called in some very hot paths inside the
columnar block builder.

This change moves Sometimes() into the on.go/off.go files where it
can be simplified for the off case. We also add some
invariants.Enabled and invariants.UseFinalizers checks (which are
constant, unlike function calls) in hot paths.

CockroachDataBlockWriter/AlphaLen=4,Prefix=8,Shared=2,KeysPerPrefix=2,valueLen=8-10  75.3µs ± 1%  74.4µs ± 0%  -1.19%  (p=0.000 n=8+8)

@RaduBerinde RaduBerinde added the o-perf-efficiency Related to performance efficiency label Dec 22, 2024
@RaduBerinde RaduBerinde requested a review from a team as a code owner December 22, 2024 16:04
@cockroach-teamcity
Copy link
Member

This change is Reviewable

The `invariants.Sometimes()` was not being inlined even though it
resolves to a `return false` when invariants aren't on. It looks like
the inline decision happens before some phases of dead code
elimination. This function is called in some very hot paths inside the
columnar block builder.

This change moves `Sometimes()` into the `on.go/off.go` files where it
can be simplified for the `off` case. We also add some
`invariants.Enabled` and `invariants.UseFinalizers` checks (which are
constant, unlike function calls) in hot paths.

```
CockroachDataBlockWriter/AlphaLen=4,Prefix=8,Shared=2,KeysPerPrefix=2,valueLen=8-10  75.3µs ± 1%  74.4µs ± 0%  -1.19%  (p=0.000 n=8+8)
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
o-perf-efficiency Related to performance efficiency
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants