This delays the execution of other jobs in the IDE.
Suggestions
I can think of these possible approaches, maybe they were already considered?
Using a JobGroup
This would allow to throttle the execution to a given number of jobs running simultaneously and would also allow the user to cancel all of the jobs in the group together.
Using 1 single Job
Instead of spawning several Jobs, create 1 single job and add several projects to it. This would also have the advantage of being able to cancel all updates together and wouldn’t saturate the jobs queue, allowing other jobs in the IDE to run without a (major) delay.
Addtional question: setting up the develoment environment to contribute patches
Is there any (official) documentation about how to set up the development environment to work on fixes? If I were able to do that, I could maybe even provide a PR.
In case of a rebase (or any Git related action changing the HEAD commit, like committing), would you be able to provide the full logs, please? You can also share it privately and/or redacted as you see fit.
I suspect the VCS-related logic is a bit flawed here, as it should only trigger one job (there is no job group in place) for all related Eclipse projects. But seeing one per Eclipse project, I guess this does not work. Or it is triggering the SonarLint Core Backend, which triggers back to the client once per project. This info is meant for the Sonar team since I can’t recall how it worked internally, haven’t touched it in years.
Regarding your question of setting up a development environment, there is just no straightforward way, sorry. I know that since the configuration of the repository (Eclipse target platforms) is very much oriented towards Sonar’s environment, and setting up a “fork” environment would currently mean changing all the target platform files (see HERE) and a patch that hasn’t made it upstream yet (but this is partly on me).
Nevertheless, if you need any help with that, don’t hesitate to ping me!
I will try to reproduce the error and provide the logs, as you suggested. The problem is that this issue only happens sporadically, so I might need a while to reproduce it. One thing I am pretty sure of is that the computation happens on projects that haven’t been affected by the rebase. A colleague of mine showed me the effective changes he was rebasing and he had only changed 8 files in 3 projects, but the calculation had been running for almost 30 minutes and in several hundreds of projects (I can’t tell you the exact number because we didn’t know about the Sonar logs).
One thing though: the problem I described originally is not about the fact that the issue markers are being calculated for projects that haven’t been changed by the rebase, it is about the fact that the calculations spawn several jobs to do the calculations. I would like to point that out because IMO that is also something that could be improved. Unless there is some specific reason for which several jobs need to be spawned separately?
I will come back to you once I have the logs.
Any other tips about how to trigger the issue or how to better debug it are highly welcome!
I am still unable to reproduce the error, but here is a proposal to convert the job to a singleton, just like PDE does with the org.eclipse.pde.internal.core.ClasspathContainerState.UpdateClasspathsJob:
IssuesMarkerUpdateJob
/*
* SonarLint for Eclipse
* Copyright (C) SonarSource Sàrl
* sonarlint@sonarsource.com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
package org.sonarlint.eclipse.core.internal.jobs;
import java.net.URI;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Queue;
import java.util.Set;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.atomic.AtomicReference;
import org.eclipse.core.resources.ResourcesPlugin;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.ICoreRunnable;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.Status;
import org.eclipse.core.runtime.SubMonitor;
import org.sonarlint.eclipse.core.SonarLintLogger;
import org.sonarlint.eclipse.core.internal.SonarLintCorePlugin;
import org.sonarlint.eclipse.core.internal.preferences.SonarLintGlobalConfiguration;
import org.sonarlint.eclipse.core.internal.utils.SonarLintUtils;
import org.sonarlint.eclipse.core.resource.ISonarLintProject;
import org.sonarsource.sonarlint.core.rpc.protocol.client.issue.RaisedIssueDto;
public class IssuesMarkerUpdateJob extends AbstractSonarJob {
public static final class IssuesMarkerUpdateRequest { // This could be a record
private final ISonarLintProject project;
private final Map<URI, List<RaisedIssueDto>> issuesByFileUri;
private final boolean issuesAreOnTheFly;
public IssuesMarkerUpdateRequest(ISonarLintProject project, Map<URI, List<RaisedIssueDto>> issuesByFileUri, boolean issuesAreOnTheFly) {
super();
this.project = project;
this.issuesByFileUri = issuesByFileUri;
this.issuesAreOnTheFly = issuesAreOnTheFly;
}
}
public static final IssuesMarkerUpdateJob INSTANCE = new IssuesMarkerUpdateJob();
private final Queue<IssuesMarkerUpdateRequest> workQueue = new ConcurrentLinkedQueue<>();
// Prevent instantiation of this singleton from the outside.
private IssuesMarkerUpdateJob() {
super("Update issues markers for projects");
// By setting a lower priority, chances are that several requests are going
// to be processed together since they may not come in batches but one after
// the other in a short period of time.
setPriority(DECORATE);
}
public void addAll(Collection<IssuesMarkerUpdateRequest> request) {
workQueue.addAll(request);
schedule();
}
public void add(IssuesMarkerUpdateRequest request) {
workQueue.add(request);
schedule();
}
@Override
protected IStatus doRun(IProgressMonitor jobMonitor) throws CoreException {
SubMonitor monitor = SubMonitor.convert(jobMonitor, "Update issues markers for projects", 100);
List<IssuesMarkerUpdateRequest> requests = new ArrayList<>();
IssuesMarkerUpdateRequest request;
while ((request = workQueue.poll()) != null) {
requests.add(request);
}
monitor.setWorkRemaining(requests.size());
AtomicReference<IStatus> status = new AtomicReference<>(Status.OK_STATUS);
Set<ISonarLintProject> toNotify = new HashSet<>();
// Update in one single workspace operation, it's faster
ResourcesPlugin.getWorkspace().run((ICoreRunnable) m -> {
for (IssuesMarkerUpdateRequest req : requests) {
if (monitor.isCanceled()) {
status.set(Status.CANCEL_STATUS);
}
updateProject(req.project, req.issuesByFileUri, req.issuesAreOnTheFly, monitor.newChild(1));
toNotify.add(req.project);
}
}, monitor);
if (!status.get().equals(Status.CANCEL_STATUS)) {
// Send one notification with all projects
SonarLintCorePlugin.getAnalysisListenerManager().notifyListeners(() -> toNotify);
}
return status.get();
}
private void updateProject(
ISonarLintProject project,
Map<URI, List<RaisedIssueDto>> issuesByFileUri,
boolean issuesAreOnTheFly,
IProgressMonitor monitor)
throws CoreException {
int countAllIssues = issuesByFileUri.values().stream().mapToInt(List::size).sum();
SonarLintLogger.get().info("Found " + countAllIssues + " issue(s) on project '" + project.getName() + "'");
// To access the preference service only once and not per issue
var issuesIncludingResolved = SonarLintGlobalConfiguration.issuesIncludingResolved();
var issuesOnlyNewCode = SonarLintGlobalConfiguration.issuesOnlyNewCode();
// If the project connection offers changing the status on anticipated issues (SonarQube 10.2+) we can enable the
// context menu option on the markers.
var viableForStatusChange = SonarLintUtils.checkProjectSupportsAnticipatedStatusChange(project);
for (var entry : issuesByFileUri.entrySet()) {
var slFile = SonarLintUtils.findFileFromUri(entry.getKey());
if (slFile != null) {
SonarLintMarkerUpdater.createOrUpdateMarkers(slFile, entry.getValue(), issuesAreOnTheFly, issuesIncludingResolved, issuesOnlyNewCode, viableForStatusChange);
}
}
}
}
The key points are:
The job is a singleton i.e. no instantiation from outside the class
There is a workQueue where the callers simply add requests
The queue is drained and all requests are processed in one single WorkspaceOperation
The job can be canceled at any time, in which case no notification is sent (I’m not sure if this is the correct behavior, maybe a notification with the already altered projects should be sent in any case, for which one only has to get rid of the if (!status.get().equals(Status.CANCEL_STATUS)) and always call SonarLintCorePlugin.getAnalysisListenerManager().notifyListeners(() -> toNotify);)
Would you kindly consider testing and integrating this change into your code? I am still unable to compile the workspace so creating a patch and testing it in production is difficult, but maybe this is something that you guys can easily test and improve.
Hi @fedejeanne, thanks for sharing. Just so you know, this is flagged for the devs to have a look whenever they have the chance, thanks for your patience!
Thanks for the detailed analysis and the proposed fix, it matched what we were seeing in the logs. A single IssueChanged event was triggering ~1,800 separate IssuesMarkerUpdateJob runs in a large workspace, which flooded the Eclipse job scheduler.
We’re looking at implementing a fix where requests are queued and processed in one workspace operation, with a single listener notification at the end. We’ll let you know once we make progress on it.
).