Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix massive performance regression for projects using correct module graphs #411

Merged
merged 17 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/project-roots.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ export default class ProjectRoots {
ignore: ['**/.git/**', '**/bower_components/**', '**/dist/**', '**/node_modules/**', '**/tmp/**'],
});

logInfo(`ELS: Found ${roots.length} roots for ${workspaceRoot}`);

const start = Date.now();

for (const rootPath of roots) {
const filePath = path.join(workspaceRoot, rootPath);
const fullPath = path.dirname(filePath);
Expand All @@ -103,6 +107,8 @@ export default class ProjectRoots {
await this.onProjectAdd(fullPath);
}
}

logInfo(`ELS: iterating roots took ${Date.now() - start}ms`);
}

async initialize(workspaceRoot: string) {
Expand All @@ -122,6 +128,8 @@ export default class ProjectRoots {
if (this.projects.has(projectPath)) {
const project = this.projects.get(projectPath) as Project;

logInfo(`Project already existed at ${projectPath}`);

return {
initIssues: project.initIssues,
providers: project.providers,
Expand Down Expand Up @@ -154,12 +162,16 @@ export default class ProjectRoots {
};
}

logInfo(`Initializing new project at ${projectPath} with ${this.localAddons.length} ELS addons.`);

const project = new Project(projectPath, this.localAddons, info);

const start = Date.now();

await project.initialize(this.server);

this.projects.set(projectPath, project);
logInfo(`Ember CLI project added at ${projectPath}`);
logInfo(`Ember CLI project added at ${projectPath}. (took ${Date.now() - start}ms)`);
await project.init(this.server);

return {
Expand Down
10 changes: 9 additions & 1 deletion src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,12 +168,14 @@ export default class Server {
hoverProvider!: HoverProvider;
codeActionProvider!: CodeActionProvider;
async executeInitializers() {
logInfo('UELS: executeInitializers');
logInfo('ELS: executeInitializers');
logInfo(`ELS: ${this.initializers.length} initializers`);

for (const initializer of this.initializers) {
await initializer();
}

logInfo(`ELS: clearing initializers because they've been initialized`);
this.initializers = [];
}
private onInitialized() {
Expand All @@ -190,6 +192,10 @@ export default class Server {

if (this.lazyInit) {
try {
/**
* Don't await this so that "format on save" is not blocked.
* We still need to run the initializers so that we have hinting in component files though.
*/
await this.executeInitializers();
NullVoxPopuli marked this conversation as resolved.
Show resolved Hide resolved
} catch (e) {
logError(e);
Expand Down Expand Up @@ -524,6 +530,8 @@ export default class Server {
this.initializers.push(async () => {
await this.projectRoots.initialize(rootPath as string);

logInfo(`Found ${workspaceFolders?.length ?? 0} workspace folders for ${rootPath}`);

if (workspaceFolders && Array.isArray(workspaceFolders)) {
for (const folder of workspaceFolders) {
const folderPath = URI.parse(folder.uri).fsPath;
Expand Down
12 changes: 11 additions & 1 deletion src/utils/addon-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
} from './layout-helpers';
import { TextDocument } from 'vscode-languageserver-textdocument';
import * as path from 'path';
import { log, logInfo, logError, safeStringify } from './logger';
import { log, logInfo, logError, safeStringify, instrumentTime } from './logger';
import Server from '../server';
import ASTPath from './../glimmer-utils';
import DAGMap from 'dag-map';
Expand Down Expand Up @@ -188,7 +188,13 @@ function requireUncached(module: string) {
}

export async function collectProjectProviders(root: string, addons: string[]): Promise<ProjectProviders> {
const time = instrumentTime(`collectProjectProviders(${root})`);

time.log(`Starting`);
const [projectAddonsRoots, projectInRepoAddonsRoots] = await Promise.all([getProjectAddonsRoots(root), getProjectInRepoAddonsRoots(root)]);

time.log(`found roots`);

const roots = addons
.concat([root])
.concat(projectAddonsRoots, projectInRepoAddonsRoots)
Expand Down Expand Up @@ -226,6 +232,8 @@ export async function collectProjectProviders(root: string, addons: string[]): P
}
}

time.log(`found ELS addons`);

const result: {
definitionProviders: DefinitionResolveFunction[];
referencesProviders: ReferenceResolveFunction[];
Expand Down Expand Up @@ -332,6 +340,8 @@ export async function collectProjectProviders(root: string, addons: string[]): P
}
});

time.log(`finished crawling dagMap`);

return result;
}

Expand Down
44 changes: 44 additions & 0 deletions src/utils/layout-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,14 @@ import { clean, coerce, valid } from 'semver';
import { BaseProject } from '../base-project';
import { fsProvider } from '../fs-provider';
import walkAsync from './walk-async';
import { instrumentTime } from './logger';

// const GLOBAL_REGISTRY = ['primitive-name'][['relatedFiles']];

// Don't traverse dependencies we've already seen.
// correct package graph can sort of throw us in to cycles if we don't keep track of this.
const SEEN = new Set<string>();
NullVoxPopuli marked this conversation as resolved.
Show resolved Hide resolved

export const ADDON_CONFIG_KEY = 'ember-language-server';

export async function asyncFilter<T>(arr: T[], predicate: (value: unknown) => Promise<boolean | undefined>): Promise<T[]> {
Expand Down Expand Up @@ -196,6 +201,16 @@ export function cached(_proto: unknown, prop: string, desc: PropertyDescriptor)

async function getRecursiveInRepoAddonRoots(root: string, roots: string[]) {
const packageData = await asyncGetPackageJSON(root);

// names are required for packages
if (!packageData.name) return [];

if (SEEN.has(packageData.name)) {
NullVoxPopuli marked this conversation as resolved.
Show resolved Hide resolved
return [];
}

SEEN.add(packageData.name);

const emberAddonPaths: string[] = (packageData['ember-addon'] && packageData['ember-addon'].paths) || [];

if (roots.length) {
Expand All @@ -213,6 +228,15 @@ async function getRecursiveInRepoAddonRoots(root: string, roots: string[]) {
for (const validRoot of validPaths) {
const packInfo = await asyncGetPackageJSON(validRoot);

// names are required for packages
if (!packInfo.name) continue;

if (SEEN.has(packInfo.name)) {
continue;
}

SEEN.add(packInfo.name);

// we don't need to go deeper if package itself not an ember-addon or els-extension
if (!isEmberAddon(packInfo) && !hasEmberLanguageServerExtension(packInfo)) {
continue;
Expand All @@ -234,8 +258,12 @@ async function getRecursiveInRepoAddonRoots(root: string, roots: string[]) {
}

export async function getProjectInRepoAddonsRoots(root: string): Promise<string[]> {
const time = instrumentTime(`getProjectInRepoAddonsRoots(${root})`);

const roots: string[] = await getRecursiveInRepoAddonRoots(root, []);

time.log(`finished getRecursiveInRepoAddonRoots`);

return Array.from(new Set(roots));
}

Expand Down Expand Up @@ -274,8 +302,16 @@ export async function isGlimmerXProject(root: string) {
}

export async function getProjectAddonsRoots(root: string, resolvedItems: string[] = [], packageFolderName = 'node_modules') {
const time = instrumentTime(`getProjectInRepoAddonsRoots(${root})`);

const pack = await asyncGetPackageJSON(root);

if (!pack.name) return [];

if (SEEN.has(pack.name)) return [];

SEEN.add(pack.name);

if (resolvedItems.length) {
if (!isEmberAddon(pack)) {
return [];
Expand Down Expand Up @@ -304,6 +340,12 @@ export async function getProjectAddonsRoots(root: string, resolvedItems: string[
for (const rootItem of roots) {
const packInfo = packages[roots.indexOf(rootItem)];

if (!packInfo.name) continue;

if (SEEN.has(packInfo.name)) continue;

SEEN.add(packInfo.name);

// we don't need to go deeper if package itself not an ember-addon or els-extension
if (!isEmberAddon(packInfo) && !hasEmberLanguageServerExtension(packInfo)) {
continue;
Expand All @@ -321,6 +363,8 @@ export async function getProjectAddonsRoots(root: string, resolvedItems: string[
}
}

time.log(`Finished looping over ${roots.length} roots`);

return recursiveRoots;
}

Expand Down
18 changes: 18 additions & 0 deletions src/utils/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,24 @@ export function logInfo(str: string) {
}
}

export function instrumentTime(label: string) {
let last = Date.now();

return {
reset: () => {
last = Date.now();
},
log: (msg: string) => {
const now = Date.now();
const diff = now - last;

last = now;

logInfo(`[${label}] +${diff}ms :: ${msg}`);
},
};
}

export function setConsole(item: RemoteConsole | null) {
remoteConsole = item;
}
Expand Down
31 changes: 31 additions & 0 deletions test/__snapshots__/integration-test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -7339,6 +7339,37 @@ Object {
}
`;

exports[`integration async fs enabled: true Go to definition works for all supported cases to children route from application outlet 2`] = `
Object {
"addonsMeta": Array [],
"registry": Object {
"routePath": Object {
"application": Array [
"app/templates/application.hbs",
],
"foo": Array [
"app/templates/foo.hbs",
],
},
},
"response": Array [
Object {
"range": Object {
"end": Object {
"character": 0,
"line": 0,
},
"start": Object {
"character": 0,
"line": 0,
},
},
"uri": "/app/templates/foo.hbs",
},
],
}
`;

exports[`integration async fs enabled: true Go to definition works for all supported cases to children route from meaningful outlet 1`] = `
Object {
"addonsMeta": Array [],
Expand Down
Loading