Skip to content

Commit

Permalink
GH-2638: Improve progress logging during initialisation and check per…
Browse files Browse the repository at this point in the history
…formance (#2640)

* integrate scan workspace into build progress

* only respect direct dependencies to reduce computation

* pass import declaration to infer importing project

* keep checks in sync

* if not found during validation, check workspace wide for project

* clean-up

* renamings

* fixes

* fix
  • Loading branch information
mmews-n4 authored Aug 23, 2024
1 parent 17eee72 commit 19d9042
Show file tree
Hide file tree
Showing 28 changed files with 455 additions and 316 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import org.apache.log4j.LogManager;
import org.apache.log4j.Logger;
import org.eclipse.lsp4j.InitializeParams;
import org.eclipse.lsp4j.InitializedParams;
import org.eclipse.lsp4j.WorkspaceFolder;
import org.eclipse.n4js.cli.N4jscConsole;
import org.eclipse.n4js.cli.N4jscException;
Expand Down Expand Up @@ -84,6 +83,11 @@ public N4jscExitState start() throws Exception {

params.setWorkspaceFolders(Collections.singletonList(new WorkspaceFolder(baseDir.toURI().toString())));
languageServer.initialize(params).get();
LanguageServerFrontend frontend = languageServer.getFrontend();
frontend.initialized(languageServer.getBaseDir());
N4jscConsole.println("Scanning workspace ...");
workspaceManager.createWorkspaceConfig();

throwIfNoProjectsFound();
verbosePrintAllProjects();

Expand Down Expand Up @@ -120,9 +124,10 @@ private void performCompile() {
callback.resetCounters();
}
Stopwatch compilationTime = Stopwatch.createStarted();
LanguageServerFrontend frontend = languageServer.getFrontend();
try (Measurement m = N4JSDataCollectors.dcBuild.getMeasurement()) {
languageServer.initialized(new InitializedParams());
languageServer.joinServerRequests();
frontend.rebuildWorkspace(false);
frontend.join();
}
printCompileResults(compilationTime.stop());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public void notifyProgress(ProgressParams params) {
}
if (notification instanceof WorkDoneProgressReport) {
WorkDoneProgressReport report = (WorkDoneProgressReport) notification;
String msg = String.format("%2d%% %dERRs %dWRNs %s", report.getPercentage(), errCount, wrnCount,
String msg = String.format(" %2d%% %dERRs %dWRNs %s", report.getPercentage(), errCount, wrnCount,
report.getMessage());
N4jscConsole.setInfoLine(msg);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ private String createLibValue() {
sb.append("[");
sb.append("\"es2019\", \"es2020\"");
for (ProjectReference requiredLibRef : projectConfig.getProjectDescription().getRequiredRuntimeLibraries()) {
N4JSPackageName requiredLibName = requiredLibRef.getN4JSProjectName();
N4JSPackageName requiredLibName = requiredLibRef.getN4JSPackageName();
ImmutableSet<String> dtsLibNames = N4JSGlobals.N4JS_DTS_LIB_CORRESPONDENCE.get(requiredLibName);
for (String dtsLibName : dtsLibNames) {
sb.append(", \"");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import java.util.Set;

import org.eclipse.n4js.packagejson.projectDescription.ProjectDescription;
import org.eclipse.n4js.packagejson.projectDescription.ProjectType;
import org.eclipse.n4js.workspace.N4JSProjectConfigSnapshot;
import org.eclipse.n4js.xtext.workspace.BuildOrderFactory;
Expand All @@ -30,12 +31,18 @@ protected Set<String> getDependencies(ProjectConfigSnapshot pc) {
N4JSProjectConfigSnapshot n4pcs = pc instanceof N4JSProjectConfigSnapshot
? (N4JSProjectConfigSnapshot) pc
: null;
if (n4pcs != null && n4pcs.getType() == ProjectType.PLAINJS) {
// ignore dependencies of plain-JS projects to non-n4js-lib projects, because
// (1) they are irrelevant for the build order of N4JS code,
// (2) npm packages sometimes declare cyclic dependencies (and we must not show errors for those cycles)
Set<String> n4jsDeps = Sets.filter(dependencies, dep -> n4pcs.isKnownDependency(dep));
return n4jsDeps;

if (n4pcs != null) {
ProjectDescription prjDescr = n4pcs.getProjectDescription();

// keep in sync with ProjectDiscoveryHelper#findDependencies()
if (!prjDescr.hasN4JSNature() && !prjDescr.isWorkspaceRoot() && prjDescr.getTypes() == null) {
// ignore dependencies of plain-JS projects to non-n4js-lib projects, because
// (1) they are irrelevant for the build order of N4JS code,
// (2) npm packages sometimes declare cyclic dependencies (and we must not show errors for those cycles)
Set<String> n4jsDeps = Sets.filter(dependencies, dep -> n4pcs.isKnownDependency(dep));
return n4jsDeps;
}
}
return dependencies;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ public Void refresh(ILanguageServerAccess access, CancelIndicator cancelIndicato
@ExecutableCommandHandler(N4JS_REBUILD)
public Void rebuild(ILanguageServerAccess access, CancelIndicator cancelIndicator) {
builderFrontend.clean();
builderFrontend.reinitWorkspace();
builderFrontend.rebuildWorkspace(true);
return null;
}

Expand Down Expand Up @@ -383,7 +383,7 @@ public Void installNpm(
// @formatter:on
return new CoarseGrainedChangeEvent();
};
}).whenComplete((a, b) -> builderFrontend.reinitWorkspace());
}).whenComplete((a, b) -> builderFrontend.rebuildWorkspace(true));

return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
*/
package org.eclipse.n4js.transpiler.dts.transform;

import org.eclipse.n4js.n4JS.ImportDeclaration;
import org.eclipse.n4js.transpiler.es.transform.ModuleSpecifierTransformation;
import org.eclipse.n4js.ts.types.TModule;

Expand All @@ -19,7 +20,7 @@
public class ModuleSpecifierTransformationDts extends ModuleSpecifierTransformation {

@Override
protected String getActualFileExtension(TModule targetModule) {
protected String getActualFileExtension(ImportDeclaration importingDeclIM, TModule targetModule) {
// file extensions are not required in module specifiers inside .d.ts files
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,14 +150,18 @@ private List<VariableStatement> transformImportDecl(TModule targetModule,
if (allImportDeclsForThisModule.isEmpty()) {
return Collections.emptyList();
}
if (!requiresRewrite(targetModule)) {
ImportDeclaration importingDeclIM = allImportDeclsForThisModule.get(0);
if (!requiresRewrite(importingDeclIM, targetModule)) {
return Collections.emptyList();
}

List<ImportDeclaration> importDeclsToRewrite = new ArrayList<>(allImportDeclsForThisModule);
if (exists(importDeclsToRewrite, id -> id.getModuleSpecifierForm() == ModuleSpecifierForm.PROJECT)) {
ImportDeclaration importingDeclOrigAST = (ImportDeclaration) getState().tracer
.getOriginalASTNode(importingDeclIM);

N4JSProjectConfigSnapshot targetProject = n4jsLanguageHelper.replaceDefinitionProjectByDefinedProject(
getState().resource, workspaceAccess.findProjectContaining(targetModule), true);
importingDeclOrigAST, workspaceAccess.findProjectContaining(targetModule), true);
if (targetProject != null && targetProject.getProjectDescription().hasModuleProperty()) {
// don't rewrite project imports in case the target project has a top-level property "module" in its
// package.json,
Expand Down Expand Up @@ -264,8 +268,10 @@ private void createVarDeclsOrBindings(List<ImportDeclaration> importDecls, Symbo
}
}

private boolean requiresRewrite(TModule targetModule) {
return !n4jsLanguageHelper.isES6Module(getState().index, targetModule);
private boolean requiresRewrite(ImportDeclaration importingDeclIM, TModule targetModule) {
ImportDeclaration importingDeclOrigAST = (ImportDeclaration) getState().tracer
.getOriginalASTNode(importingDeclIM);
return !n4jsLanguageHelper.isES6Module(getState().index, importingDeclOrigAST, targetModule);
}

private String computeNameForIntermediateDefaultImport(TModule targetModule) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,10 @@ public void transform() {
}

/** Returns file extension of output module */
protected String getActualFileExtension(TModule targetModule) {
return n4jsLanguageHelper.getOutputFileExtension(getState().index, targetModule);
protected String getActualFileExtension(ImportDeclaration importingDeclIM, TModule targetModule) {
ImportDeclaration importingDeclOrigAST = (ImportDeclaration) getState().tracer
.getOriginalASTNode(importingDeclIM);
return n4jsLanguageHelper.getOutputFileExtension(getState().index, importingDeclOrigAST, targetModule);
}

private void transformImportDecl(ImportDeclaration importDeclIM) {
Expand Down Expand Up @@ -160,7 +162,7 @@ private String computeModuleSpecifierForOutputCode(ImportDeclaration importDeclI
// module specifiers are always absolute in N4JS, but Javascript requires relative module
// specifiers when importing from a module within the same npm package
// --> need to create a relative module specifier here:
return createRelativeModuleSpecifier(targetModule);
return createRelativeModuleSpecifier(importDeclIM, targetModule);
}

ModuleSpecifierForm moduleSpecifierForm = importDeclIM.getModuleSpecifierForm();
Expand All @@ -177,10 +179,10 @@ private String computeModuleSpecifierForOutputCode(ImportDeclaration importDeclI
return importDeclIM.getModuleSpecifierAsText();
}

return createAbsoluteModuleSpecifier(targetProject, targetModule);
return createAbsoluteModuleSpecifier(targetProject, importDeclIM, targetModule);
}

private String createRelativeModuleSpecifier(TModule targetModule) {
private String createRelativeModuleSpecifier(ImportDeclaration importingDeclIM, TModule targetModule) {
String targetModuleSpecifier = resourceNameComputer.getCompleteModuleSpecifier(targetModule);
String[] targetModuleSpecifierSegments = targetModuleSpecifier.split("/", -1);
String targetModuleName = targetModuleSpecifierSegments[targetModuleSpecifierSegments.length - 1];
Expand All @@ -195,14 +197,16 @@ private String createRelativeModuleSpecifier(TModule targetModule) {
List<String> allSegm = new ArrayList<>(Arrays.asList(differingSegments));
allSegm.add(targetModuleName);
int goUpCount = localModulePath.length - i;
String ext = getActualFileExtension(targetModule);
String ext = getActualFileExtension(importingDeclIM, targetModule);
String result = ((goUpCount > 0) ? "../".repeat(goUpCount) : "./")
+ org.eclipse.n4js.utils.Strings.join("/", allSegm)
+ ((ext != null && !ext.isEmpty()) ? "." + ext : "");
return result;
}

private String createAbsoluteModuleSpecifier(N4JSProjectConfigSnapshot targetProject, TModule targetModule) {
private String createAbsoluteModuleSpecifier(N4JSProjectConfigSnapshot targetProject,
ImportDeclaration importingDeclIM, TModule targetModule) {

if (N4JSLanguageUtils.isMainModule(targetProject, targetModule)) {
// 'targetModule' is the main module of 'targetProject', so we can use a project import:
return getActualProjectName(targetProject).toString();
Expand Down Expand Up @@ -236,7 +240,7 @@ private String createAbsoluteModuleSpecifier(N4JSProjectConfigSnapshot targetPro
String targetModuleSpecifier = resourceNameComputer.getCompleteModuleSpecifier(targetModule);
sb.append(targetModuleSpecifier);

String ext = getActualFileExtension(targetModule);
String ext = getActualFileExtension(importingDeclIM, targetModule);
if (ext != null && !ext.isEmpty()) {
sb.append('.');
sb.append(ext);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,10 @@ public class LanguageServerFrontend implements TextDocumentService, WorkspaceSer
/**
* Initialize this front-end according to the given arguments.
*/
public void initialize(InitializeParams params, URI baseDir, ILanguageServerAccess access) {
public void initialize(InitializeParams params, ILanguageServerAccess access) {
logWelcomeMessage();
workspaceFrontend.initialize(access);
textDocumentFrontend.initialize(params, access);
builderFrontend.initialize(baseDir);
}

/**
Expand All @@ -131,8 +130,8 @@ protected void logWelcomeMessage() {
/**
* Notifies the front-end that the initialization was completed.
*/
public void initialized() {
builderFrontend.initialBuild();
public void initialized(URI baseDir) {
builderFrontend.initialize(baseDir);
}

/**
Expand Down Expand Up @@ -191,7 +190,7 @@ public void didSave(DidSaveTextDocumentParams params) {

@Override
public void didChangeConfiguration(DidChangeConfigurationParams params) {
reinitWorkspace();
rebuildWorkspace();
}

@Override
Expand All @@ -214,8 +213,13 @@ public void clean() {
}

/** Triggers rebuild of the whole workspace in the background. Can be awaited by {@link #join()}. */
public void reinitWorkspace() {
builderFrontend.reinitWorkspace();
public void rebuildWorkspace() {
rebuildWorkspace(true);
}

/** Triggers rebuild of the whole workspace in the background. Can be awaited by {@link #join()}. */
public void rebuildWorkspace(boolean recreateWorkspace) {
builderFrontend.rebuildWorkspace(recreateWorkspace);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,16 +266,16 @@ public CompletableFuture<InitializeResult> initialize(InitializeParams params) {
if (initializeParams != null) {
throw new IllegalStateException("This language server has already been initialized.");
}
URI baseDir = getBaseDir(params);
if (languagesRegistry.getExtensionToFactoryMap().isEmpty()) {
throw new IllegalStateException(
"No Xtext languages have been registered. Please make sure you have added the languages\'s setup class in \'/META-INF/services/org.eclipse.xtext.ISetup\'");
}
this.initializeParams = params;
URI baseDir = getBaseDir();

Stopwatch sw = Stopwatch.createStarted();
LOG.info("Start server initialization in workspace directory " + baseDir);
lsFrontend.initialize(initializeParams, baseDir, access);
lsFrontend.initialize(initializeParams, access);
LOG.info("Server initialization done after " + sw);

initializeResult = new InitializeResult();
Expand Down Expand Up @@ -392,7 +392,9 @@ protected Optional<List<String>> getSupportedCodeActionKinds() {

@Override
public void initialized(InitializedParams params) {
lsFrontend.initialized();
URI baseDir = getBaseDir();
lsFrontend.initialized(baseDir);
lsFrontend.rebuildWorkspace();
}

@Deprecated
Expand All @@ -416,8 +418,8 @@ private URI deprecatedToBaseDir2(InitializeParams params) {
/**
* Compute the base directory.
*/
protected URI getBaseDir(InitializeParams params) {
List<WorkspaceFolder> workspaceFolders = params.getWorkspaceFolders();
public URI getBaseDir() {
List<WorkspaceFolder> workspaceFolders = initializeParams.getWorkspaceFolders();
if (workspaceFolders != null && !workspaceFolders.isEmpty()) {
// TODO: Support multiple workspace folders
WorkspaceFolder workspaceFolder = workspaceFolders.get(0);
Expand All @@ -427,7 +429,7 @@ protected URI getBaseDir(InitializeParams params) {
}
return uriExtensions.toUri(workspaceFolder.getUri());
}
return deprecatedToBaseDir2(params);
return deprecatedToBaseDir2(initializeParams);
}

@SuppressWarnings("hiding")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,6 @@ public void initialize(URI newBaseDir) {
workspaceManager.initialize(newBaseDir);
}

/**
* Trigger an initial build in the background.
*/
public void initialBuild() {
asyncRunBuildTask("initialBuild", workspaceBuilder::createInitialBuildTask);
}

/**
* Trigger a clean operation in the background. Afterwards, all previously build artifacts have been removed. This
* includes index data, source2generated mappings, cached issues and persisted index state. A subsequent build is
Expand All @@ -104,8 +97,8 @@ public void clean() {
/**
* Triggers rebuild of the whole workspace
*/
public void reinitWorkspace() {
asyncRunBuildTask("reinitWorkspace", workspaceBuilder::createReinitialBuildTask);
public void rebuildWorkspace(boolean recreateWorkspace) {
asyncRunBuildTask("rebuildWorkspace", () -> workspaceBuilder.createFullBuildTask(recreateWorkspace));
}

/**
Expand Down
Loading

0 comments on commit 19d9042

Please sign in to comment.