Skip to content

Commit

Permalink
sql/catalog/descs: optimize access to the system db
Browse files Browse the repository at this point in the history
We tend to access the system database a lot and we never cache it in the
lease manager. Before this patch, we'd always copy and re-allocate a pair
of them. There's no need to do that.

```
name                                           old time/op    new time/op    delta
FlowSetup/vectorize=true/distribute=true-16       141µs ± 3%     132µs ± 4%  -6.14%  (p=0.000 n=19+20)
FlowSetup/vectorize=true/distribute=false-16      138µs ± 4%     128µs ± 3%  -7.36%  (p=0.000 n=19+19)
FlowSetup/vectorize=false/distribute=true-16      134µs ± 2%     123µs ± 3%  -7.78%  (p=0.000 n=20+20)
FlowSetup/vectorize=false/distribute=false-16     129µs ± 3%     119µs ± 3%  -7.51%  (p=0.000 n=20+19)

name                                           old alloc/op   new alloc/op   delta
FlowSetup/vectorize=true/distribute=true-16      38.1kB ± 2%    36.5kB ± 2%  -4.19%  (p=0.000 n=18+18)
FlowSetup/vectorize=true/distribute=false-16     36.2kB ± 0%    34.6kB ± 0%  -4.32%  (p=0.000 n=17+17)
FlowSetup/vectorize=false/distribute=true-16     42.6kB ± 0%    41.1kB ± 0%  -3.63%  (p=0.000 n=18+16)
FlowSetup/vectorize=false/distribute=false-16    41.0kB ± 0%    39.4kB ± 0%  -3.76%  (p=0.000 n=16+18)

name                                           old allocs/op  new allocs/op  delta
FlowSetup/vectorize=true/distribute=true-16         368 ± 0%       342 ± 0%  -7.07%  (p=0.000 n=16+17)
FlowSetup/vectorize=true/distribute=false-16        354 ± 0%       328 ± 0%  -7.34%  (p=0.000 n=18+17)
FlowSetup/vectorize=false/distribute=true-16        337 ± 0%       312 ± 1%  -7.63%  (p=0.000 n=19+19)
FlowSetup/vectorize=false/distribute=false-16       325 ± 0%       299 ± 0%  -8.00%  (p=0.000 n=17+18)
```

Release note: None
  • Loading branch information
ajwerner committed Oct 26, 2021
1 parent 31ccb1c commit 1a06e43
Showing 1 changed file with 21 additions and 0 deletions.
21 changes: 21 additions & 0 deletions pkg/sql/catalog/descs/uncommitted_descriptors.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@ package descs

import (
"github.com/cockroachdb/cockroach/pkg/sql/catalog"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/dbdesc"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/lease"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/nstree"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/systemschema"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/typedesc"
"github.com/cockroachdb/cockroach/pkg/util/iterutil"
"github.com/cockroachdb/errors"
Expand Down Expand Up @@ -113,6 +115,7 @@ func (ud *uncommittedDescriptors) add(mut catalog.MutableDescriptor) (catalog.De
// checkOut checks out an uncommitted mutable descriptor for use in the
// transaction. This descriptor should later be checked in again.
func (ud *uncommittedDescriptors) checkOut(id descpb.ID) (catalog.MutableDescriptor, error) {
ud.maybeInitialize()
entry := ud.descs.GetByID(id)
if entry == nil {
return nil, errors.NewAssertionErrorWithWrappedErrf(
Expand Down Expand Up @@ -170,6 +173,7 @@ func maybeRefreshCachedFieldsOnTypeDescriptor(

// getByID looks up an uncommitted descriptor by ID.
func (ud *uncommittedDescriptors) getByID(id descpb.ID) catalog.Descriptor {
ud.maybeInitialize()
entry := ud.descs.GetByID(id)
if entry == nil {
return nil
Expand All @@ -187,6 +191,7 @@ func (ud *uncommittedDescriptors) getByID(id descpb.ID) catalog.Descriptor {
func (ud *uncommittedDescriptors) getByName(
dbID descpb.ID, schemaID descpb.ID, name string,
) (hasKnownRename bool, desc catalog.Descriptor) {
ud.maybeInitialize()
// Walk latest to earliest so that a DROP followed by a CREATE with the same
// name will result in the CREATE being seen.
if got := ud.descs.GetByName(dbID, schemaID, name); got != nil {
Expand All @@ -202,6 +207,7 @@ func (ud *uncommittedDescriptors) getByName(
func (ud *uncommittedDescriptors) iterateNewVersionByID(
fn func(entry catalog.NameEntry, originalVersion lease.IDVersion) error,
) error {
ud.maybeInitialize()
return ud.descs.IterateByID(func(entry catalog.NameEntry) error {
mut := entry.(*uncommittedDescriptor).mutable
if mut.IsNew() || !mut.IsUncommittedVersion() {
Expand All @@ -214,6 +220,7 @@ func (ud *uncommittedDescriptors) iterateNewVersionByID(
func (ud *uncommittedDescriptors) iterateImmutableByID(
fn func(imm catalog.Descriptor) error,
) error {
ud.maybeInitialize()
return ud.descs.IterateByID(func(entry catalog.NameEntry) error {
return fn(entry.(*uncommittedDescriptor).immutable)
})
Expand Down Expand Up @@ -262,3 +269,17 @@ func (ud *uncommittedDescriptors) hasUncommittedTypes() (has bool) {
})
return has
}

var systemUncommittedDatabase = func() *uncommittedDescriptor {
ud, err := makeUncommittedDescriptor(dbdesc.NewBuilder(systemschema.SystemDB.DatabaseDesc()).BuildExistingMutable())
if err != nil {
panic(err)
}
return ud
}()

func (ud *uncommittedDescriptors) maybeInitialize() {
if ud.descs.Len() == 0 {
ud.descs.Upsert(systemUncommittedDatabase)
}
}

0 comments on commit 1a06e43

Please sign in to comment.