Skip to content

Commit

Permalink
build(awslint): Cache L1 constructs in the CFN resource linter. (#27290)
Browse files Browse the repository at this point in the history
Incremental change to improve the performance of awslint. `CfnResourceReflection.findByName()` was checking the doc tags of every class in the assembly on each call, taking 170s for `aws-cdk-lib`. This change caches the tagged classes, eliminating the list traversal and taking `findByName` down to ~2s for the same package.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
mikewrighton authored Sep 27, 2023
1 parent 6616026 commit 68895ff
Showing 1 changed file with 26 additions and 4 deletions.
30 changes: 26 additions & 4 deletions packages/awslint/lib/rules/cfn-resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,29 @@ import { CoreTypes } from './core-types';
import { ResourceReflection } from './resource';
import { Linter } from '../linter';

const cfnResourceTagName = 'cloudformationResource';

// this linter verifies that we have L2 coverage. it finds all "Cfn" classes and verifies
// that we have a corresponding L1 class for it that's identified as a resource.
export const cfnResourceLinter = new Linter(a => CfnResourceReflection.findAll(a));

// Cache L1 constructs per type system.
const l1ConstructCache = new Map<reflect.TypeSystem, Map<string, reflect.ClassType>>();

function cacheL1ConstructsForTypeSystem(sys: reflect.TypeSystem) {
if (!l1ConstructCache.has(sys)) {
l1ConstructCache.set(sys, new Map<string, reflect.ClassType>());

for (const cls of sys.classes) {
const cfnResourceTag = cls.docs.customTag(cfnResourceTagName);

if (cfnResourceTag) {
l1ConstructCache.get(sys)?.set(cfnResourceTag?.toLocaleLowerCase()!, cls);
}
}
}
}

cfnResourceLinter.add({
code: 'resource-class',
message: 'every resource must have a resource class (L2), add \'@resource %s\' to its docstring',
Expand All @@ -24,10 +43,13 @@ export class CfnResourceReflection {
* @param fullName first two components are case-insensitive (e.g. `aws::s3::Bucket` is equivalent to `Aws::S3::Bucket`)
*/
public static findByName(sys: reflect.TypeSystem, fullName: string) {
for (const cls of sys.classes) {
if (cls.docs.customTag('cloudformationResource')?.toLocaleLowerCase() === fullName.toLocaleLowerCase()) {
return new CfnResourceReflection(cls);
}
if (!l1ConstructCache.has(sys)) {
cacheL1ConstructsForTypeSystem(sys);
}

const cls = l1ConstructCache.get(sys)?.get(fullName.toLowerCase());
if (cls) {
return new CfnResourceReflection(cls);
}

return undefined;
Expand Down

0 comments on commit 68895ff

Please sign in to comment.