Skip to content

sql/schema: re-organize the UnleasableSystemDescriptors set#74353

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
nvb:nvanbenschoten/assortedPerf16
Jan 3, 2022
Merged

sql/schema: re-organize the UnleasableSystemDescriptors set#74353
craig[bot] merged 1 commit intocockroachdb:masterfrom
nvb:nvanbenschoten/assortedPerf16

Conversation

@nvb
Copy link
Copy Markdown
Contributor

@nvb nvb commented Dec 31, 2021

This commit re-organizes the static maps we use to perform lookups into the
UnleasableSystemDescriptors. It splits the UnleasableSystemDescriptors map
into two, one optimized for leasedDescriptors.getByName and one optimized for
leasedDescriptors.getByID.

In a 100% read workload, leasedDescriptors accesses (getByName and
getByID) were responsible for 4.39 of total CPU utilization.

Screen Shot 2021-12-30 at 7 35 38 PM

Within this, about 0.33% of total CPU utilization was spent directly in
these functions. This change shouldn't make a large difference (less than
0.2%), but it should improve things slightly. Since it also improves
readability, it seems worthwhile.

      File: cockroach
Type: cpu
Time: Dec 30, 2021 at 10:57pm (UTC)
Duration: 30.15s, Total samples = 74.99s (248.72%)
Active filters:
   focus=leasedDescriptors\).getBy
Showing nodes accounting for 3.29s, 4.39% of 74.99s total
----------------------------------------------------------+-------------
      flat  flat%   sum%        cum   cum%   calls calls% + context
----------------------------------------------------------+-------------
                                             0.14s   100% |   github.com/cockroachdb/cockroach/pkg/sql/catalog/descs.(*Collection).getByName /go/src/github.com/cockroachdb/cockroach/pkg/sql/catalog/descs/descriptor.go:199
     0.10s  0.13%  0.13%      0.14s  0.19%                | github.com/cockroachdb/cockroach/pkg/sql/catalog/descs.(*leasedDescriptors).getByName /go/src/github.com/cockroachdb/cockroach/pkg/sql/catalog/descs/leased_descriptors.go:102
                                             0.03s 21.43% |   github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb.(*TableDescriptor).GetParentID /go/src/github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb/structured.pb.go:2347
                                             0.01s  7.14% |   github.com/cockroachdb/cockroach/pkg/sql/catalog/dbdesc.(*immutable).GetParentID /go/src/github.com/cockroachdb/cockroach/pkg/sql/catalog/dbdesc/database_desc.go:93
----------------------------------------------------------+-------------
                                             0.14s   100% |   github.com/cockroachdb/cockroach/pkg/sql/catalog/descs.(*Collection).getByName /go/src/github.com/cockroachdb/cockroach/pkg/sql/catalog/descs/descriptor.go:199
     0.04s 0.053%  0.19%      0.14s  0.19%                | github.com/cockroachdb/cockroach/pkg/sql/catalog/descs.(*leasedDescriptors).getByName /go/src/github.com/cockroachdb/cockroach/pkg/sql/catalog/descs/leased_descriptors.go:101
                                             0.03s 21.43% |   runtime.mapiternext /usr/local/go/src/runtime/map.go:851
                                             0.02s 14.29% |   runtime.mapiterinit /usr/local/go/src/runtime/map.go:821
                                             0.01s  7.14% |   runtime.duffzero /usr/local/go/src/runtime/duff_amd64.s:95
                                             0.01s  7.14% |   runtime.mapiterinit /usr/local/go/src/runtime/map.go:832
                                             0.01s  7.14% |   runtime.mapiterinit /usr/local/go/src/runtime/map.go:848
                                             0.01s  7.14% |   runtime.mapiternext /usr/local/go/src/runtime/map.go:898
                                             0.01s  7.14% |   runtime.mapiternext /usr/local/go/src/runtime/map.go:972
----------------------------------------------------------+-------------

This commit re-organizes the static maps we use to perform lookups into the
`UnleasableSystemDescriptors`. It splits the `UnleasableSystemDescriptors` map
into two, one optimized for `leasedDescriptors.getByName` and one optimized for
`leasedDescriptors.getByID`.

In a 100% read workload, `leasedDescriptors` accesses (`getByName` and
`getByID`) were responsible for **4.39** of total CPU utilization.

Within this, about **0.33%** of total CPU utilization was spent directly in
these functions. This change shouldn't make a large difference (less than
**0.2%**), but it should improve things slightly. Since it also improves
readability, it seems worthwhile.

```
      File: cockroach
Type: cpu
Time: Dec 30, 2021 at 10:57pm (UTC)
Duration: 30.15s, Total samples = 74.99s (248.72%)
Active filters:
   focus=leasedDescriptors\).getBy
Showing nodes accounting for 3.29s, 4.39% of 74.99s total
----------------------------------------------------------+-------------
      flat  flat%   sum%        cum   cum%   calls calls% + context
----------------------------------------------------------+-------------
                                             0.14s   100% |   github.com/cockroachdb/cockroach/pkg/sql/catalog/descs.(*Collection).getByName /go/src/github.com/cockroachdb/cockroach/pkg/sql/catalog/descs/descriptor.go:199
     0.10s  0.13%  0.13%      0.14s  0.19%                | github.com/cockroachdb/cockroach/pkg/sql/catalog/descs.(*leasedDescriptors).getByName /go/src/github.com/cockroachdb/cockroach/pkg/sql/catalog/descs/leased_descriptors.go:102
                                             0.03s 21.43% |   github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb.(*TableDescriptor).GetParentID /go/src/github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb/structured.pb.go:2347
                                             0.01s  7.14% |   github.com/cockroachdb/cockroach/pkg/sql/catalog/dbdesc.(*immutable).GetParentID /go/src/github.com/cockroachdb/cockroach/pkg/sql/catalog/dbdesc/database_desc.go:93
----------------------------------------------------------+-------------
                                             0.14s   100% |   github.com/cockroachdb/cockroach/pkg/sql/catalog/descs.(*Collection).getByName /go/src/github.com/cockroachdb/cockroach/pkg/sql/catalog/descs/descriptor.go:199
     0.04s 0.053%  0.19%      0.14s  0.19%                | github.com/cockroachdb/cockroach/pkg/sql/catalog/descs.(*leasedDescriptors).getByName /go/src/github.com/cockroachdb/cockroach/pkg/sql/catalog/descs/leased_descriptors.go:101
                                             0.03s 21.43% |   runtime.mapiternext /usr/local/go/src/runtime/map.go:851
                                             0.02s 14.29% |   runtime.mapiterinit /usr/local/go/src/runtime/map.go:821
                                             0.01s  7.14% |   runtime.duffzero /usr/local/go/src/runtime/duff_amd64.s:95
                                             0.01s  7.14% |   runtime.mapiterinit /usr/local/go/src/runtime/map.go:832
                                             0.01s  7.14% |   runtime.mapiterinit /usr/local/go/src/runtime/map.go:848
                                             0.01s  7.14% |   runtime.mapiternext /usr/local/go/src/runtime/map.go:898
                                             0.01s  7.14% |   runtime.mapiternext /usr/local/go/src/runtime/map.go:972
----------------------------------------------------------+-------------
```
@nvb nvb requested review from a team and postamar December 31, 2021 00:46
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Copy link
Copy Markdown
Contributor

@ajwerner ajwerner left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @postamar)

@nvb
Copy link
Copy Markdown
Contributor Author

nvb commented Jan 3, 2022

TFTR!

bors r=ajwerner

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Jan 3, 2022

Build succeeded:

@craig craig bot merged commit 5669b9d into cockroachdb:master Jan 3, 2022
@postamar
Copy link
Copy Markdown

postamar commented Jan 4, 2022

Very nice! Thanks for doing this.

@nvb nvb deleted the nvanbenschoten/assortedPerf16 branch January 6, 2022 16:45
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.

4 participants