Skip to content

Improve RequiredPluginsInitializer.initialize #1711

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.

Sign up for GitHub

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

Merged
merks merged 1 commit into from
Apr 3, 2025
Merged

Improve RequiredPluginsInitializer.initialize #1711

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
78 changes: 66 additions & 12 deletions ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/RequiredPluginsInitializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,15 @@
*******************************************************************************/
package org.eclipse.pde.internal.core;

import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Set;

import org.eclipse.core.resources.IProject;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IPath;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.Status;
import org.eclipse.core.runtime.jobs.Job;
Expand All @@ -35,26 +41,64 @@ public class RequiredPluginsInitializer extends ClasspathContainerInitializer {
}
});

private static final DeferredClasspathContainerInitializerJob deferredClasspathContainerInitializerJob = new DeferredClasspathContainerInitializerJob();

private static class DeferredClasspathContainerInitializerJob extends Job {

private final Set<IJavaProject> projects = new LinkedHashSet<>();

public DeferredClasspathContainerInitializerJob() {
Comment thread
merks marked this conversation as resolved.
// This name is not displayed to a user.
super("DeferredClasspathContainerInitializerJob"); //$NON-NLS-1$
setSystem(true);
}

public synchronized void initialize(IJavaProject project) {
Copy link
Copy Markdown
Contributor

@laeubi laeubi Apr 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it needs to be public?

Copy link
Copy Markdown
Contributor Author

@merks merks Apr 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its more performant for what that matters; i.e., no bridge method to make it visible... It's in a private class so it doesn't matter either way...

if (projects.add(project)) {
schedule();
}
}

private synchronized IJavaProject[] consumeProjects() {
try {
return projects.toArray(IJavaProject[]::new);
} finally {
projects.clear();
}
}

@Override
protected IStatus run(IProgressMonitor monitor) {
try {
setupClasspath(consumeProjects());
} catch (JavaModelException e) {
PDECore.log(e);
}
return Status.OK_STATUS;
}

@Override
public boolean belongsTo(Object family) {
return family == PluginModelManager.class;
}
}

@Override
public void initialize(IPath containerPath, IJavaProject javaProject) throws CoreException {
if (Job.getJobManager().isSuspended()) {
// if the jobmanager is currently suspended we can't use the
// schedule/join pattern here, instead we must retry the requested
// action once jobs are enabled again, this will the possibly
// trigger a rebuild if required or notify other listeners.
Job job = Job.create(PDECoreMessages.PluginModelManager_InitializingPluginModels, m -> {
setupClasspath(javaProject);
});
job.setSystem(true);
job.schedule();
deferredClasspathContainerInitializerJob.initialize(javaProject);
} else {
setupClasspath(javaProject);
}
}

protected void setupClasspath(IJavaProject javaProject) throws JavaModelException {
IProject project = javaProject.getProject();
// The first project to be built may initialize the PDE models, potentially long running, so allow cancellation
protected static void setupClasspath(IJavaProject... javaProjects) throws JavaModelException {
Copy link
Copy Markdown
Contributor

@laeubi laeubi Apr 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really useful to change this method for the case we actually like to avoid? How many projects are there to be collected that its worth to optimize here?

Copy link
Copy Markdown
Contributor Author

@merks merks Apr 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it can be quite a large number, 18 and 35 I've seen; all timing related of course. The overhead downstream is massive, spawning huge chains of events that if you step over it steps for a very long time. Given we are batching the projects it makes sense to me to batch the whole deal so that all the classpath updates happen as a single workspace operation that could kick off a single build. In fact there is so much overhead that when I added this optimization the problem at the JDT side of things came back because of the fewer events bubbling through the system.

// The first project to be built may initialize the PDE models,
// potentially long running, so allow cancellation
PluginModelManager manager = PDECore.getDefault().getModelManager();
if (!manager.isInitialized()) {
initPDEJob.schedule();
Expand All @@ -63,11 +107,21 @@ protected void setupClasspath(IJavaProject javaProject) throws JavaModelExceptio
} catch (InterruptedException e) {
}
}
if (project.exists() && project.isOpen()) {
IPluginModelBase model = manager.findModel(project);
JavaCore.setClasspathContainer(PDECore.REQUIRED_PLUGINS_CONTAINER_PATH, new IJavaProject[] { javaProject },
new IClasspathContainer[] { new RequiredPluginsClasspathContainer(model, project) }, null);

Map<IJavaProject, RequiredPluginsClasspathContainer> classPathContainers = new LinkedHashMap<>();
for (IJavaProject javaProject : javaProjects) {
IProject project = javaProject.getProject();
if (project.exists() && project.isOpen()) {
IPluginModelBase model = manager.findModel(project);
RequiredPluginsClasspathContainer requiredPluginsClasspathContainer = new RequiredPluginsClasspathContainer(
model, project);
classPathContainers.put(javaProject, requiredPluginsClasspathContainer);
}
}

JavaCore.setClasspathContainer(PDECore.REQUIRED_PLUGINS_CONTAINER_PATH,
classPathContainers.keySet().toArray(IJavaProject[]::new),
classPathContainers.values().toArray(IClasspathContainer[]::new), null);
}

@Override
Expand Down
Loading