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

drop init time cost of apd from 0.530ms to 0.002ms #137

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mvdan
Copy link

@mvdan mvdan commented Aug 6, 2024

(see commit messages - please do not squash)

@mvdan
Copy link
Author

mvdan commented Aug 6, 2024

cc @josharian :)

@mvdan mvdan force-pushed the init branch 2 times, most recently from 8b430e3 to 019e443 Compare August 6, 2024 23:55
As measured by benchinit plus benchstat, this shaves off about 0.46ms
and 1200 allocs from the package's init time, which is fairly noticeable
for any downstream binary's ability to quickly get to its main func.

For example, cuelang.org/go/cmd/cue takes about 4ms to reach main
due to initializing Go packages, so cockroackdb/apd/v3 was to blame
for about 12% of that slowness.

    goos: linux
    goarch: amd64
    cpu: AMD Ryzen 7 PRO 5850U with Radeon Graphics
                              │     old      │                new                 │
                              │    sec/op    │   sec/op     vs base               │
    GithubComCockroachdbApdV3   533.11µ ± 3%   76.33µ ± 3%  -85.68% (p=0.000 n=8)

                              │      old      │                 new                 │
                              │     B/op      │     B/op      vs base               │
    GithubComCockroachdbApdV3   564.05Ki ± 0%   43.91Ki ± 0%  -92.21% (p=0.000 n=8)

                              │     old     │                new                 │
                              │  allocs/op  │  allocs/op   vs base               │
    GithubComCockroachdbApdV3   2.461k ± 0%   1.222k ± 0%  -50.35% (p=0.000 n=8)
Thanks to the previous commit delaying the making of global constants,
the tables below are no longer unconditionally needed at init time,
but only as the first API calls come in from user code.

On top of the 0.46ms saving from the previous commit, this change
saves us 0.07ms and over 1100 allocations from init time,
bringing apd/v3's init time cost down to just 0.002ms.

    goos: linux
    goarch: amd64
    cpu: AMD Ryzen 7 PRO 5850U with Radeon Graphics
                              │     mid      │                 new                 │
                              │    sec/op    │    sec/op     vs base               │
    GithubComCockroachdbApdV3   76.325µ ± 3%   2.116µ ± 12%  -97.23% (p=0.000 n=8)

                              │      mid      │                 new                 │
                              │     B/op      │     B/op      vs base               │
    GithubComCockroachdbApdV3   43.914Ki ± 0%   1.539Ki ± 0%  -96.50% (p=0.000 n=8)

                              │     mid      │                new                │
                              │  allocs/op   │ allocs/op   vs base               │
    GithubComCockroachdbApdV3   1222.00 ± 0%   66.00 ± 0%  -94.60% (p=0.000 n=8)
@mvdan
Copy link
Author

mvdan commented Aug 20, 2024

Friendly ping @nvanbenschoten :) Let me know if I should do anything else to help this along. Note that #139 caused conflicts so I dropped the CI changes from this PR for now.

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.

1 participant