Skip to content

Commit

Permalink
Changes to benchmarking to make v1 work with v0 and a new target that…
Browse files Browse the repository at this point in the history
… exemplifies symbol caching
  • Loading branch information
markwpearce committed Sep 24, 2021
1 parent 2403a57 commit 6a1fc98
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 8 deletions.
20 changes: 17 additions & 3 deletions benchmarks/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class Runner {
this.project = options.project;
this.quick = options.quick;
this.profile = options.profile;
this.tar = options.tar;
}
run() {
this.downloadFiles();
Expand Down Expand Up @@ -57,15 +58,21 @@ class Runner {
}
}

buildCurrentTarball() {
buildCurrent() {
const bscDir = path.resolve(__dirname, '..');
console.log('benchmark: build current brighterscript');
this.npmSync(['run', 'build'], {
cwd: bscDir
});
return 'file:' + path.resolve(bscDir);
}

buildCurrentTarball() {
const bscDir = path.resolve(__dirname, '..');
this.buildCurrent();
console.log('benchmark: pack current brighterscript');
const filename = this.npmSync(['pack'], { cwd: bscDir, stdio: 'pipe' }).stdout.toString().trim();
return path.resolve(bscDir, filename);
return path.resolve(filename);
}

/**
Expand All @@ -79,13 +86,15 @@ class Runner {
fsExtra.emptyDirSync(nodeModulesDir);

const dependencies = {};
console.log(`benchmark: using versions: ${this.versions}`);
for (let i = 0; i < this.versions.length; i++) {
const version = this.versions[i];
const name = `brighterscript${i + 1}`;

//if the version is "current", then make a local copy of the package from the dist folder to install (because npm link makes things slower)
if (version === 'local') {
dependencies[name] = this.buildCurrentTarball();
const localVersion = this.tar ? this.buildCurrentTarball() : this.buildCurrent();
dependencies[name] = localVersion;
} else {
dependencies[name] = `npm:brighterscript@${version}`;
}
Expand Down Expand Up @@ -202,6 +211,11 @@ let options = yargs
description: 'Enable nodejs profiling of each benchmark run',
default: false
})
.option('tar', {
type: 'boolean',
description: 'use a npm-packed tarball for local files instead of using the files directly',
default: true
})
.strict()
.check(argv => {
const idx = argv.versions.indexOf('latest');
Expand Down
29 changes: 29 additions & 0 deletions benchmarks/targets/hover-brs.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
module.exports = async (suite, name, brighterscript, projectPath, options) => {
const { ProgramBuilder } = brighterscript;

const builder = new ProgramBuilder();
//run the first run so we we can focus the test on validate
await builder.run({
cwd: projectPath,
createPackage: false,
copyToStaging: false,
//disable diagnostic reporting (they still get collected)
diagnosticFilters: ['**/*'],
logLevel: 'error'
});
const files = Object.values(builder.program.files).filter(x => ['.brs', '.bs'].includes(x.extension));
if (files.length === 0) {
console.log('[hover-brs] No brs|bs files found in program');
return;
}

suite.add(name, () => {
for (const file of files) {
// Hover every 20th token
for (let i = 0; i < file.parser.tokens.length; i += 20) {
const token = file.parser.tokens[i];
file.getHover(token.range.end);
}
}
}, options);
};
3 changes: 2 additions & 1 deletion benchmarks/targets/parse-brs.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ module.exports = async (suite, name, brighterscript, projectPath, options) => {
}
suite.add(name, (deferred) => {
const promises = [];
const setFileFuncName = builder.program.setFile ? 'setFile' : 'addOrReplaceFile';
for (const file of files) {
promises.push(
builder.program.addOrReplaceFile(file.pkgPath, file.fileContents)
builder.program[setFileFuncName](file.pkgPath, file.fileContents)
);
}
// eslint-disable-next-line @typescript-eslint/no-floating-promises
Expand Down
15 changes: 13 additions & 2 deletions src/Scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ export class Scope {
public isValidated: boolean;

protected cache = new Cache();
protected symbolCache = new Map<Token, TokenSymbolLookup>();

public get dependencyGraphKey() {
return this._dependencyGraphKey;
Expand All @@ -72,6 +71,15 @@ export class Scope {
return this.getClassFileLink(className, containingNamespace)?.item;
}

/**
* A cache of a map of tokens -> TokenSymbolLookups, which are the result of getSymbolTypeFromToken()
* Sometimes the lookup of symbols may take a while if there are lazyTypes or multiple tokens in a chain
* By caching the result of this lookup, subsequent lookups of the same tokens are quicker
*/
protected get symbolCache() {
return this.cache.getOrAdd('symbolCache', () => new Map<Token, TokenSymbolLookup>());
}

/**
* Get a class and its containing file by the class name
* @param className - The class name, including the namespace of the class if possible
Expand Down Expand Up @@ -152,7 +160,6 @@ export class Scope {
return ancestors;
}


public hasCachedSymbolForToken(token: Token) {
return this.symbolCache.has(token);
}
Expand All @@ -161,6 +168,10 @@ export class Scope {
return this.symbolCache.get(token);
}

/**
* Stores a TokenSymbolLookup object associated with a token
* @returns the "lookupValue" that was passed in
*/
public setCachedSymbolForToken(token: Token, lookupValue: TokenSymbolLookup): TokenSymbolLookup {
this.symbolCache.set(token, lookupValue);
return lookupValue;
Expand Down
5 changes: 3 additions & 2 deletions src/files/BrsFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -832,8 +832,9 @@ export class BrsFile {
if (!scope) {
return undefined;
}
if (scope.hasCachedSymbolForToken(currentToken)) {
return scope.getCachedSymbolForToken(currentToken);
const cachedSymbolData = scope.getCachedSymbolForToken(currentToken);
if (cachedSymbolData) {
return cachedSymbolData;
}
const tokenChainResponse = this.parser.getTokenChain(currentToken);
if (tokenChainResponse.includesUnknowableTokenType) {
Expand Down

0 comments on commit 6a1fc98

Please sign in to comment.