catalog/descs: consider optimizing getNonVirtualDescriptorID

[yuzefovich]

Here is a snippet of alloc_objects profile from one of the nodes (on v23.1.17):

ROUTINE ======================== github.com/cockroachdb/cockroach/pkg/sql/catalog/descs.(*Collection).getNonVirtualDescriptorID.func5 in github.com/cockroachdb/cockroach/pkg/sql/catalog/descs/descriptor.go
2630107188 2630107188 (flat, cum)  1.82% of Total
         .          .    556:           return haltLookups, ud.GetID(), nil
         .          .    557:       }
         .          .    558:       return continueLookups, descpb.InvalidID, nil
         .          .    559:   }
         .          .    560:   lookupStoreCacheID := func() (continueOrHalt, descpb.ID, error) {
1316932341 1316932341    561:       ni := descpb.NameInfo{ParentID: parentID, ParentSchemaID: parentSchemaID, Name: name}
1313174847 1313174847    562:       if tc.isShadowedName(ni) {
         .          .    563:           return continueLookups, descpb.InvalidID, nil
         .          .    564:       }
         .          .    565:       if tc.cr.IsNameInCache(&ni) {
         .          .    566:           if e := tc.cr.Cache().LookupNamespaceEntry(&ni); e != nil {
         .          .    567:               return haltLookups, e.GetID(), nil

I'm guessing we could easily avoid these objects escaping.

Jira issue: CRDB-37175

Epic CRDB-42584

[rafiss]

@yuzefovich could you remind me, what's the approach to take to make these objects not escape?

[yuzefovich]

IIUC there are two allocations that we see in this snippet:

• descpb.NameInfo escapes to the heap because it is passed to methods that take an interface as an argument. IIUC it is a barrier for the compiler to see whether the value outlives the current call frame.
• we incur another allocation due to interface boxing.

So the solution would be to ideally de-virtualize some of the methods used in this function so that we can use descpb.NameInfo directly and the compiler could see that it doesn't escape (this would also removing the interface boxing). It's easy to de-virtualize the call to isShadowedName (by manually inlining the relevant logic), but IsNameInCache and LookupNamespaceEntry don't look as trivial. It might be the case that we cannot avoid these allocations.

[nvanbenschoten]

One of the two heap allocations here is eliminated by https://github.com/cockroachdb/cockroach/pull/134216.