Skip to content

Commit

Permalink
sql/schema: re-organize the UnleasableSystemDescriptors set
Browse files Browse the repository at this point in the history
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
----------------------------------------------------------+-------------
```
  • Loading branch information
nvanbenschoten committed Dec 31, 2021
1 parent cc22c45 commit 362242f
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 17 deletions.
10 changes: 3 additions & 7 deletions pkg/sql/catalog/descs/leased_descriptors.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,8 @@ func (ld *leasedDescriptors) getByName(
return cached.(lease.LeasedDescriptor).Underlying(), false, nil
}

for _, d := range systemschema.UnleasableSystemDescriptors {
if parentID == d.GetParentID() &&
parentSchemaID == d.GetParentSchemaID() &&
name == d.GetName() {
return nil, true, nil
}
if systemschema.IsUnleasableSystemDescriptorByName(parentID, parentSchemaID, name) {
return nil, true, nil
}

readTimestamp := txn.ReadTimestamp()
Expand All @@ -128,7 +124,7 @@ func (ld *leasedDescriptors) getByID(
return cached.(lease.LeasedDescriptor).Underlying(), false, nil
}

if _, isUnleasable := systemschema.UnleasableSystemDescriptors[id]; isUnleasable {
if systemschema.IsUnleasableSystemDescriptorByID(id) {
return nil, true, nil
}

Expand Down
63 changes: 53 additions & 10 deletions pkg/sql/catalog/systemschema/system.go
Original file line number Diff line number Diff line change
Expand Up @@ -2291,23 +2291,66 @@ var (
}}
},
)
)

// UnleasableSystemDescriptors contains the system descriptors which cannot
// be leased. This includes the lease table itself, among others.
UnleasableSystemDescriptors = func(s []catalog.Descriptor) map[descpb.ID]catalog.Descriptor {
m := make(map[descpb.ID]catalog.Descriptor, len(s))
for _, d := range s {
m[d.GetID()] = d
}
return m
}([]catalog.Descriptor{
type descRefByName struct {
parentID descpb.ID
parentSchemaID descpb.ID
name string
}

var (
// UnleasableSystemDescriptors contains the system descriptors which
// cannot be leased. This includes the lease table itself, among others.
UnleasableSystemDescriptors = []catalog.Descriptor{
SystemDB,
LeaseTable,
DescriptorTable,
NamespaceTable,
RangeEventTable,
})
}

unleasableSystemDescriptorsByID = func(s []catalog.Descriptor) map[descpb.ID]struct{} {
m := make(map[descpb.ID]struct{}, len(s))
for _, d := range s {
m[d.GetID()] = struct{}{}
}
return m
}(UnleasableSystemDescriptors)

unleasableSystemDescriptorsByName = func(s []catalog.Descriptor) map[descRefByName]struct{} {
m := make(map[descRefByName]struct{}, len(s))
for _, d := range s {
m[descRefByName{
parentID: d.GetParentID(),
parentSchemaID: d.GetParentSchemaID(),
name: d.GetName(),
}] = struct{}{}
}
return m
}(UnleasableSystemDescriptors)
)

// IsUnleasableSystemDescriptorByID returns whether the specified descriptor is
// a member of the UnleasableSystemDescriptors set, given an ID.
func IsUnleasableSystemDescriptorByID(id descpb.ID) bool {
_, ok := unleasableSystemDescriptorsByID[id]
return ok
}

// IsUnleasableSystemDescriptorByName returns whether the specified descriptor
// is a member of the UnleasableSystemDescriptors set, given a database, schema,
// and name.
func IsUnleasableSystemDescriptorByName(
parentID descpb.ID, parentSchemaID descpb.ID, name string,
) bool {
_, ok := unleasableSystemDescriptorsByName[descRefByName{
parentID: parentID,
parentSchemaID: parentSchemaID,
name: name,
}]
return ok
}

// SpanConfigurationsTableName represents system.span_configurations.
var SpanConfigurationsTableName = tree.NewTableNameWithSchema("system", tree.PublicSchemaName, tree.Name(catconstants.SpanConfigurationsTableName))

0 comments on commit 362242f

Please sign in to comment.