Skip to content

Commit

Permalink
perf: limit initSchema calls from openapi.IsNamespaceScoped (#5076)
Browse files Browse the repository at this point in the history
* test: add openapi.IsNamespaceScoped benchmark

Add a benchmark test for IsNamespaceScoped performance when the default
schema is in use.

* perf: limit initSchema calls from openapi.IsNamespaceScoped

Avoid calling initSchema from openapi.IsNamespaceScoped when possible.
Work done in #4152 introduced a precomputed namespace scope map based on
the default built-in schema. This commit extends that work by avoiding
calls to initSchema when a resource is not found in the precomputed map
and the default built-in schema is in use. In those cases, there is no
benefit to calling initSchema since the precomputed map is exactly what
will be calculated by parsing the default built-in schema.

* fix: delay parsing of default built-in schema

When namespace scope can be determined by the precomputed map but the
type is not present in the precomputed map, delay the parsing of the
default built-in schema.

If the schema to be initialized is the default built-in schema and the
type is not in the precomputed map, then the type will not be found in
the default built-in schema. There is no need to parse the default
built-in schema for that answer; its parsing may be delayed until it
is needed for some other purpose.

In cases where the schema is used solely for namespace scope checks, the
schema might not ever be parsed. Skipping the parsing reduces both
execution time and memory use.

* fix: correct openapi.go's schemaNotParsed value

openapiData initializes with defaultBuiltInSchemaParseStatus set to 0,
so schemaNotParsed should have 0 as its value.
  • Loading branch information
ephesused authored Sep 8, 2023
1 parent f81765b commit 985835f
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 9 deletions.
65 changes: 56 additions & 9 deletions kyaml/openapi/openapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,16 @@ var (
customSchema []byte //nolint:gochecknoglobals
)

// schemaParseStatus is used in cases when a schema should be parsed, but the
// parsing may be delayed to a later time.
type schemaParseStatus uint32

const (
schemaNotParsed schemaParseStatus = iota
schemaParseDelayed
schemaParsed
)

// openapiData contains the parsed openapi state. this is in a struct rather than
// a list of vars so that it can be reset from tests.
type openapiData struct {
Expand All @@ -57,13 +67,17 @@ type openapiData struct {
// is namespaceable or not
namespaceabilityByResourceType map[yaml.TypeMeta]bool

// noUseBuiltInSchema stores whether we want to prevent using the built-n
// noUseBuiltInSchema stores whether we want to prevent using the built-in
// Kubernetes schema as part of the global schema
noUseBuiltInSchema bool

// schemaInit stores whether or not we've parsed the schema already,
// so that we only reparse the when necessary (to speed up performance)
schemaInit bool

// defaultBuiltInSchemaParseStatus stores the parse status of the default
// built-in schema.
defaultBuiltInSchemaParseStatus schemaParseStatus
}

type format string
Expand Down Expand Up @@ -387,18 +401,45 @@ func GetSchema(s string, schema *spec.Schema) (*ResourceSchema, error) {
// be true if the resource is namespace-scoped, and false if the type is
// cluster-scoped.
func IsNamespaceScoped(typeMeta yaml.TypeMeta) (bool, bool) {
if res, f := precomputedIsNamespaceScoped[typeMeta]; f {
return res, true
if isNamespaceScoped, found := precomputedIsNamespaceScoped[typeMeta]; found {
return isNamespaceScoped, found
}
if isInitSchemaNeededForNamespaceScopeCheck() {
initSchema()
}
return isNamespaceScopedFromSchema(typeMeta)
}

func isNamespaceScopedFromSchema(typeMeta yaml.TypeMeta) (bool, bool) {
initSchema()
isNamespaceScoped, found := globalSchema.namespaceabilityByResourceType[typeMeta]
return isNamespaceScoped, found
}

// isInitSchemaNeededForNamespaceScopeCheck returns true if initSchema is needed
// to ensure globalSchema.namespaceabilityByResourceType is fully populated for
// cases where a custom or non-default built-in schema is in use.
func isInitSchemaNeededForNamespaceScopeCheck() bool {
schemaLock.Lock()
defer schemaLock.Unlock()

if globalSchema.schemaInit {
return false // globalSchema already is initialized.
}
if customSchema != nil {
return true // initSchema is needed.
}
if kubernetesOpenAPIVersion == "" || kubernetesOpenAPIVersion == kubernetesOpenAPIDefaultVersion {
// The default built-in schema is in use. Since
// precomputedIsNamespaceScoped aligns with the default built-in schema
// (verified by TestIsNamespaceScopedPrecompute), there is no need to
// call initSchema.
if globalSchema.defaultBuiltInSchemaParseStatus == schemaNotParsed {
// The schema may be needed for purposes other than namespace scope
// checks. Flag it to be parsed when that need arises.
globalSchema.defaultBuiltInSchemaParseStatus = schemaParseDelayed
}
return false
}
// A non-default built-in schema is in use. initSchema is needed.
return true
}

// IsCertainlyClusterScoped returns true for Node, Namespace, etc. and
// false for Pod, Deployment, etc. and kinds that aren't recognized in the
// openapi data. See:
Expand Down Expand Up @@ -638,13 +679,19 @@ func initSchema() {
panic(fmt.Errorf("invalid schema file: %w", err))
}
} else {
if kubernetesOpenAPIVersion == "" {
if kubernetesOpenAPIVersion == "" || kubernetesOpenAPIVersion == kubernetesOpenAPIDefaultVersion {
parseBuiltinSchema(kubernetesOpenAPIDefaultVersion)
globalSchema.defaultBuiltInSchemaParseStatus = schemaParsed
} else {
parseBuiltinSchema(kubernetesOpenAPIVersion)
}
}

if globalSchema.defaultBuiltInSchemaParseStatus == schemaParseDelayed {
parseBuiltinSchema(kubernetesOpenAPIDefaultVersion)
globalSchema.defaultBuiltInSchemaParseStatus = schemaParsed
}

if err := parse(kustomizationapi.MustAsset(kustomizationAPIAssetName), JsonOrYaml); err != nil {
// this should never happen
panic(err)
Expand Down
17 changes: 17 additions & 0 deletions kyaml/openapi/openapi_benchmark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
openapi_v2 "github.com/google/gnostic-models/openapiv2"
"google.golang.org/protobuf/proto"
"sigs.k8s.io/kustomize/kyaml/openapi/kubernetesapi"
"sigs.k8s.io/kustomize/kyaml/yaml"
)

func BenchmarkProtoUnmarshal(t *testing.B) {
Expand All @@ -31,3 +32,19 @@ func BenchmarkProtoUnmarshal(t *testing.B) {
}
}
}

func BenchmarkPrecomputedIsNamespaceScoped(b *testing.B) {
testcases := map[string]yaml.TypeMeta{
"namespace scoped": {APIVersion: "apps/v1", Kind: "ControllerRevision"},
"cluster scoped": {APIVersion: "rbac.authorization.k8s.io/v1", Kind: "ClusterRole"},
"unknown resource": {APIVersion: "custom.io/v1", Kind: "Custom"},
}
for name, testcase := range testcases {
b.Run(name, func(b *testing.B) {
for i := 0; i < b.N; i++ {
ResetOpenAPI()
_, _ = IsNamespaceScoped(testcase)
}
})
}
}

0 comments on commit 985835f

Please sign in to comment.