Skip to content

Commit

Permalink
Performance boost by lifting some global lookups (#92)
Browse files Browse the repository at this point in the history
  • Loading branch information
TwitchBronBron authored Jul 14, 2023
1 parent 2fbc1e1 commit d5c4a88
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 29 deletions.
12 changes: 12 additions & 0 deletions .vscode/tasks.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,18 @@
"label": "npm: build",
"detail": "tsc"
},
{
"label": "watch",
"type": "npm",
"script": "watch",
"presentation": {
"group": "watch"
},
"isBackground": true,
"problemMatcher": [
"$tsc-watch"
]
},
{
"type": "shell",
"label": "test",
Expand Down
5 changes: 3 additions & 2 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { BsConfig, Program, DiagnosticSeverity } from 'brighterscript';
import { BsConfig, Program, DiagnosticSeverity, CompilerPlugin } from 'brighterscript';
import Linter from './Linter';
import CheckUsage from './plugins/checkUsage';
import CodeStyle from './plugins/codeStyle';
Expand Down Expand Up @@ -71,9 +71,10 @@ export interface BsLintRules {

export { Linter };

export default function factory() {
export default function factory(): CompilerPlugin {
const contextMap = new WeakMap<Program, PluginWrapperContext>();
return {
name: 'bslint',
afterProgramCreate: (program: Program) => {
const context = createContext(program);
contextMap.set(program, context);
Expand Down
78 changes: 51 additions & 27 deletions src/plugins/trackCodeFlow/varTracking.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { BscFile, FunctionExpression, BsDiagnostic, Range, isForStatement, isForEachStatement, isIfStatement, isAssignmentStatement, isNamespaceStatement, NamespaceStatement, Expression, isVariableExpression, isBinaryExpression, TokenKind, Scope, CallableContainerMap, DiagnosticSeverity, isLiteralInvalid, isWhileStatement, isClassMethodStatement, isBrsFile, isCatchStatement, isLabelStatement, isGotoStatement, NamespacedVariableNameExpression, ParseMode } from 'brighterscript';
import { BscFile, FunctionExpression, BsDiagnostic, Range, isForStatement, isForEachStatement, isIfStatement, isAssignmentStatement, isNamespaceStatement, NamespaceStatement, Expression, isVariableExpression, isBinaryExpression, TokenKind, Scope, CallableContainerMap, DiagnosticSeverity, isLiteralInvalid, isWhileStatement, isCatchStatement, isLabelStatement, isGotoStatement, NamespacedVariableNameExpression, ParseMode, util, isMethodStatement } from 'brighterscript';
import { LintState, StatementInfo, NarrowingInfo, VarInfo, VarRestriction } from '.';
import { PluginContext } from '../../util';

Expand Down Expand Up @@ -52,7 +52,7 @@ export function createVarLinter(
args.set(name.toLowerCase(), { name: name, range: p.name.range, isParam: true, isUnsafe: false, isUsed: false });
});

if (isClassMethodStatement(fun.functionStatement)) {
if (isMethodStatement(fun.functionStatement)) {
args.set('super', { name: 'super', range: null, isParam: true, isUnsafe: false, isUsed: true });
}

Expand Down Expand Up @@ -379,47 +379,48 @@ export function runDeferredValidation(
files: BscFile[],
callables: CallableContainerMap
) {
const globals = lintContext.globals;

const topLevelVars = buildTopLevelVars(scope, lintContext.globals);
const diagnostics: BsDiagnostic[] = [];
files.forEach((file) => {
const deferred = deferredValidation.get(file.pathAbsolute);
if (deferred) {
deferredVarLinter(scope, file, callables, globals, deferred, diagnostics);
deferredVarLinter(scope, file, callables, topLevelVars, deferred, diagnostics);
}
});
return diagnostics;
}

/**
* Get a list of all top level variables available in the scope
*/
function buildTopLevelVars(scope: Scope, globals: string[]) {
// lookups for namespaces, classes, enums, etc...
// to add them to the topLevel so that they don't get marked as unused.
const toplevel = new Set<string>(globals);

for (const namespace of scope.getAllNamespaceStatements()) {
toplevel.add(getRootNamespaceName(namespace).toLowerCase()); // keep root of namespace
}
for (const [, cls] of scope.getClassMap()) {
toplevel.add(cls.item.name.text.toLowerCase());
}
for (const [, enm] of scope.getEnumMap()) {
toplevel.add(enm.item.name.toLowerCase());
}
for (const [, cnst] of scope.getConstMap()) {
toplevel.add(cnst.item.name.toLowerCase());
}
return toplevel;
}

function deferredVarLinter(
scope: Scope,
file: BscFile,
callables: CallableContainerMap,
globals: string[],
toplevel: Set<string>,
deferred: ValidationInfo[],
diagnostics: BsDiagnostic[]
) {
// lookups for namespaces, classes, and enums
// to add them to the topLevel so that they don't get marked as unused.
const toplevel = new Set<string>(globals);
scope.getAllNamespaceStatements().forEach(ns => {
toplevel.add(ns.name.toLowerCase().split('.')[0]); // keep root of namespace
});
scope.getClassMap().forEach(cls => {
toplevel.add(cls.item.name.text.toLowerCase());
});
scope.getEnumMap().forEach(enm => {
toplevel.add(enm.item.name.toLowerCase());
});
scope.getConstMap().forEach(cnt => {
toplevel.add(cnt.item.name.toLowerCase());
});
if (isBrsFile(file)) {
file.parser.references.classStatements.forEach(cls => {
toplevel.add(cls.name.text.toLowerCase());
});
}

deferred.forEach(({ kind, name, local, range, namespace }) => {
const key = name?.toLowerCase();
let hasCallable = key ? callables.has(key) || toplevel.has(key) : false;
Expand Down Expand Up @@ -448,3 +449,26 @@ function deferredVarLinter(
}
});
}

/**
* Get the leftmost part of the namespace name. (i.e. `alpha` from `alpha.beta.charlie`) by walking
* up the namespace chain until we get to the very topmost namespace. Then grabbing the leftmost token's name.
*
*/
export function getRootNamespaceName(namespace: NamespaceStatement) {
// there are more concise ways to accomplish this, but this is a hot function so it's been optimized.
while (true) {
const parent = namespace.parent?.parent as NamespaceStatement;
if (isNamespaceStatement(parent)) {
namespace = parent;
} else {
break;
}
}
const result = util.getDottedGetPath(namespace.nameExpression)[0]?.name?.text;
// const name = namespace.getName(ParseMode.BrighterScript).toLowerCase();
// if (name.includes('imigx')) {
// console.log([name, result]);
// }
return result;
}

0 comments on commit d5c4a88

Please sign in to comment.