Skip to content

[performance] Import the large project #17

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

Closed
snjeza wants to merge 1 commit into from
Closed

[performance] Import the large project #17

Show file tree
Hide file tree
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
29 changes: 13 additions & 16 deletions ...lesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelElement.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@
*/
package com.salesforce.bazel.eclipse.core.model;

import static java.lang.String.format;
import static java.util.Objects.requireNonNull;
import static java.util.Optional.ofNullable;

import java.util.Optional;
import java.util.function.Supplier;

import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IPath;
import org.eclipse.core.runtime.OperationCanceledException;
import org.eclipse.core.runtime.Status;

import com.salesforce.bazel.eclipse.core.model.cache.BazelElementInfoCache;
import com.salesforce.bazel.sdk.model.BazelLabel;
Expand Down Expand Up @@ -129,24 +129,21 @@ protected final I getInfo() throws CoreException {
if (info != null) {
return info;
}

// ensure the parent is loaded
if (hasParent()) {
getParent().getInfo();
}

// loads can be potentially expensive; we synchronize on the location
var location = getLocation();
var openJob = new BazelElementOpenJob<>(location != null ? location : IPath.ROOT, this, infoCache);
Copy link
Member

@guw guw Dec 8, 2025

Choose a reason for hiding this comment

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

Eliminating the job removes transparency and visibility into the package. The job framework provides valuable insights in the Eclipse UI. I would rather like to see JDTLS improve here.

The second important thing provided by jobs framework is synchronization using ISchedulingRule. It is important that at most ONE Bazel command is executed per workspace in general. The Bazel client prevents concurrent executions usually, which can lead to unexpected pauses/waits when multiple Bazel commands are executed by different threads. The job framework (again) helps visualizing this in the UI.

try {
return openJob.open();
} catch (InterruptedException e) {
throw new OperationCanceledException("Interrupted while opening element.");
} catch (CoreException e) {
// wrap to provide better context
throw new CoreException(
Status.error(String.format("Error opening '%s': '%s'", location, e.getMessage()), e));
}
// store in cache
Supplier<I> supplier = () -> {
try {
return requireNonNull(
createInfo(),
() -> format("invalid implementation of #createInfo in %s; must not return null!", getClass()));
} catch (CoreException e) {
throw new RuntimeException(e);
}
};
return (I) infoCache.putOrGetCached(this, supplier);
}

BazelElementInfoCache getInfoCache() {
Expand Down
12 changes: 10 additions & 2 deletions ...lesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelPackage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@

import java.nio.file.Path;
import java.util.List;
import java.util.Map;
import java.util.Objects;

import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IPath;
import org.eclipse.core.runtime.Status;

import com.salesforce.bazel.sdk.command.querylight.Target;
import com.salesforce.bazel.sdk.model.BazelLabel;

/**
Expand Down Expand Up @@ -71,6 +73,7 @@ public static boolean isBuildFileName(String fileName) {
private final BazelWorkspace parent;
private final BazelLabel label;
private final IPath packagePath;
private Map<String, Target> targets;

BazelPackage(BazelWorkspace parent, IPath packagePath) throws NullPointerException, IllegalArgumentException {
this.packagePath =
Expand All @@ -86,8 +89,9 @@ protected BazelPackageInfo createInfo() throws CoreException {
throw new CoreException(
Status.error(format("Package '%s' does not exist in workspace '%s'!", label, parent.getName())));
}

var targets = BazelPackageInfo.queryForTargets(this, getCommandExecutor());
if (targets == null) {
targets = BazelPackageInfo.queryForTargets(this, getCommandExecutor());
}
return new BazelPackageInfo(buildFile, this, targets);
}

Expand Down Expand Up @@ -344,4 +348,8 @@ public void rediscoverBazelProject() throws CoreException {
getInfo();
}

public void setTargets(Map<String, Target> targets) {
this.targets = targets;
}

}
36 changes: 9 additions & 27 deletions ...orce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelPackageInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@
*/
package com.salesforce.bazel.eclipse.core.model;

import static com.salesforce.bazel.eclipse.core.BazelCoreSharedContstants.BAZEL_NATURE_ID;
import static com.salesforce.bazel.eclipse.core.model.BazelProject.hasOwnerPropertySetForLabel;
import static com.salesforce.bazel.eclipse.core.model.BazelProject.hasWorkspaceRootPropertySetToLocation;
import static java.lang.String.format;
import static java.util.stream.Collectors.joining;

Expand Down Expand Up @@ -55,20 +52,8 @@ public final class BazelPackageInfo extends BazelElementInfo {
* @throws CoreException
*/
static IProject findProject(BazelPackage bazelPackage) throws CoreException {
var workspaceRoot = bazelPackage.getBazelWorkspace().getLocation();
// we don't care about the actual project name - we look for the property
var projects = getEclipseWorkspaceRoot().getProjects();
for (IProject project : projects) {
if (project.isAccessible() // is open
&& project.hasNature(BAZEL_NATURE_ID) // is a Bazel project
&& hasWorkspaceRootPropertySetToLocation(project, workspaceRoot) // belongs to the workspace root
&& hasOwnerPropertySetForLabel(project, bazelPackage.getLabel()) // represents the target
) {
return project;
}
}

return null;
var bazelWorkspace = bazelPackage.getBazelWorkspace();
return bazelWorkspace.getInfo().getProject(bazelPackage.getLabel());
}

static Map<String, Target> queryForTargets(BazelPackage bazelPackage,
Expand Down Expand Up @@ -138,7 +123,7 @@ static Map<BazelPackage, Map<String, Target>> queryForTargets(BazelWorkspace baz
private final BazelPackage bazelPackage;
private final Map<String, Target> indexOfTargetInfoByTargetName;

private final BazelProject bazelProject;
private BazelProject bazelProject;

private BazelVisibility defaultVisibility;

Expand All @@ -147,22 +132,19 @@ static Map<BazelPackage, Map<String, Target>> queryForTargets(BazelWorkspace baz
this.buildFile = buildFile;
this.bazelPackage = bazelPackage;
this.indexOfTargetInfoByTargetName = indexOfTargetInfoByTargetName;

// find project is expensive, do it only once when loading a package
// (https://github.com/eclipseguru/bazel-eclipse/issues/8)
var project = findProject(bazelPackage);
if (project != null) {
bazelProject = new BazelProject(project, bazelPackage.getModel());
} else {
bazelProject = null;
}
}

public BazelPackage getBazelPackage() {
return bazelPackage;
}

public BazelProject getBazelProject() throws CoreException {
// find project is expensive, do it only once when loading a package
// (https://github.com/eclipseguru/bazel-eclipse/issues/8)
var project = findProject(bazelPackage);
if (project != null) {
bazelProject = new BazelProject(project, bazelPackage.getModel());
}
if (bazelProject != null) {
return bazelProject;
}
Expand Down
28 changes: 5 additions & 23 deletions ...force.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelTargetInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@
*/
package com.salesforce.bazel.eclipse.core.model;

import static com.salesforce.bazel.eclipse.core.BazelCoreSharedContstants.BAZEL_NATURE_ID;
import static com.salesforce.bazel.eclipse.core.model.BazelProject.hasOwnerPropertySetForLabel;
import static com.salesforce.bazel.eclipse.core.model.BazelProject.hasWorkspaceRootPropertySetToLocation;
import static java.lang.String.format;
import static java.util.Objects.requireNonNull;
import static java.util.stream.Collectors.toList;
Expand Down Expand Up @@ -46,26 +43,14 @@ public final class BazelTargetInfo extends BazelElementInfo {
* @throws CoreException
*/
static IProject findProject(BazelTarget bazelTarget) throws CoreException {
var workspaceRoot = bazelTarget.getBazelWorkspace().getLocation();
// we don't care about the actual project name - we look for the property
var projects = getEclipseWorkspaceRoot().getProjects();
for (IProject project : projects) {
if (project.isAccessible() // is open
&& project.hasNature(BAZEL_NATURE_ID) // is a Bazel project
&& hasWorkspaceRootPropertySetToLocation(project, workspaceRoot) // belongs to the workspace root
&& hasOwnerPropertySetForLabel(project, bazelTarget.getLabel()) // represents the target
) {
return project;
}
}

return null;
var bazelWorkspace = bazelTarget.getBazelWorkspace();
return bazelWorkspace.getInfo().getProject(bazelTarget.getLabel());
}

private final BazelTarget bazelTarget;
private final String targetName;
private final Target target;
private final BazelProject bazelProject;
private BazelProject bazelProject;
private volatile BazelRuleAttributes ruleAttributes;

private List<IPath> ruleOutput;
Expand All @@ -86,18 +71,15 @@ public BazelTargetInfo(String targetName, BazelTarget bazelTarget, BazelPackageI
getTargetName(),
packageInfo.getBazelPackage().getLabel())));
}
}

public BazelProject getBazelProject() throws CoreException {
// find project is expensive, do it only once when loading a package
// (https://github.com/eclipseguru/bazel-eclipse/issues/8)
var project = findProject(bazelTarget);
if (project != null) {
bazelProject = new BazelProject(project, bazelTarget.getModel());
} else {
bazelProject = null;
}
}

public BazelProject getBazelProject() throws CoreException {
if (bazelProject != null) {
return bazelProject;
}
Expand Down
89 changes: 87 additions & 2 deletions ...sforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelWorkspace.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,25 @@
import static com.salesforce.bazel.eclipse.core.BazelCoreSharedContstants.FILE_NAME_REPO_BAZEL;
import static com.salesforce.bazel.eclipse.core.BazelCoreSharedContstants.FILE_NAME_WORKSPACE;
import static com.salesforce.bazel.eclipse.core.BazelCoreSharedContstants.FILE_NAME_WORKSPACE_BAZEL;
import static com.salesforce.bazel.eclipse.core.model.BazelPackageInfo.queryForTargets;
import static java.lang.String.format;
import static java.nio.file.Files.isDirectory;
import static java.nio.file.Files.isRegularFile;
import static java.util.Objects.requireNonNull;
import static java.util.function.Predicate.not;
import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toList;

import java.nio.file.Path;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.function.Predicate;
import java.util.stream.Stream;

import org.eclipse.core.resources.IProject;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IPath;
import org.eclipse.core.runtime.Status;
Expand All @@ -43,6 +46,8 @@
import com.salesforce.bazel.eclipse.core.projectview.BazelProjectView;
import com.salesforce.bazel.sdk.BazelVersion;
import com.salesforce.bazel.sdk.command.BazelBinary;
import com.salesforce.bazel.sdk.command.BazelQueryForTargetProtoCommand;
import com.salesforce.bazel.sdk.command.querylight.Target;
import com.salesforce.bazel.sdk.model.BazelLabel;

/**
Expand Down Expand Up @@ -218,6 +223,11 @@ public boolean exists() {
return isDirectory(path) && (findWorkspaceFile(path) != null);
}

public IProject findProject(BazelPackage bazelPackage) throws CoreException {
var info = bazelPackage.getBazelWorkspace().getInfo();
return info.getProject(bazelPackage.getLabel());
}

/**
* @return a collection of all targets loaded in memory
*/
Expand Down Expand Up @@ -616,7 +626,7 @@ public void open(Collection<BazelPackage> bazelPackages) throws CoreException {
}

// open all closed projects
var targetsByPackage = queryForTargets(this, closedPackages, getCommandExecutor());
var targetsByPackage = queryForTargetsWithDependencies(this, closedPackages, getCommandExecutor());
for (BazelPackage bazelPackage : closedPackages) {
if (bazelPackage.hasInfo()) {
continue;
Expand All @@ -634,6 +644,11 @@ public void open(Collection<BazelPackage> bazelPackages) throws CoreException {
LOG.warn("Package '{}' does not have a BUILD file, skipping", bazelPackage.getLabel());
continue;
}
bazelPackage.setTargets(targets);
if (!bazelPackage.hasInfo()) {
// getting the info loads the package avoiding unnecessary double loads
bazelPackage.getInfo();
}

// Create PackageInfo directly with the batched query results
var packageInfo = new BazelPackageInfo(buildFile, bazelPackage, targets);
Expand All @@ -644,7 +659,77 @@ public void open(Collection<BazelPackage> bazelPackages) throws CoreException {
}
}

public Map<BazelPackage, Map<String, Target>> queryForTargetsWithDependencies(BazelWorkspace bazelWorkspace,
Collection<BazelPackage> bazelPackages, BazelElementCommandExecutor bazelElementCommandExecutor)
throws CoreException {
// bazel query 'kind(rule, deps(//foo:all + //bar:all))"'
Copy link
Member

@guw guw Dec 8, 2025

Choose a reason for hiding this comment

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

This looks like an interesting optimization for removing bazel query calls. Please submit as separate PR/commit

Copy link
Author

@snjeza snjeza Dec 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I have created #43


if (bazelPackages.isEmpty()) {
return Collections.emptyMap();
}
var workspaceRoot = bazelWorkspace.getLocation().toPath();
var query = bazelPackages.stream()
.map(bazelPackage -> format("//%s:all", bazelPackage.getWorkspaceRelativePath()))
.collect(joining(" + "));

Map<String, BazelPackage> bazelPackageByWorkspaceRelativePath = new HashMap<>();
bazelPackages.stream()
.forEach(p -> bazelPackageByWorkspaceRelativePath.put(p.getWorkspaceRelativePath().toString(), p));

query = "kind(rule, deps(" + query + "))";
Map<BazelPackage, Map<String, Target>> result = new HashMap<>();
LOG.debug("{}: querying Bazel for list of targets from: {}", bazelWorkspace, query);
var queryResult = bazelElementCommandExecutor.runQueryWithoutLock(
new BazelQueryForTargetProtoCommand(
workspaceRoot,
query,
true /* keep going */,
List.of("--noproto:locations", "--noproto:default_values", "--noimplicit_deps", "--notool_deps"),
format(
"Loading targets for %d %s",
bazelPackages.size(),
bazelPackages.size() == 1 ? "package" : "packages")));
for (Target target : queryResult) {
if (!target.hasRule()) {
LOG.trace("{}: ignoring target: {}", bazelWorkspace, target);
System.out.println();
continue;
}

try {
BazelLabel.validateLabelPath(target.rule().name(), true);
} catch (Exception e) {
LOG.trace("{}: ignoring target: {}", bazelWorkspace, target);
continue;
}
LOG.trace("{}: found target: {}", bazelWorkspace, target);
var targetLabel = new BazelLabel(target.rule().name());

var bazelPackage = bazelPackageByWorkspaceRelativePath.get(targetLabel.getPackagePath());
if (bazelPackage == null) {
// LOG.debug("{}: ignoring target for unknown package: {}", bazelWorkspace, targetLabel);
// continue;
var packageLabel = targetLabel.getPackageLabel();
if (bazelWorkspace.isRootedAtThisWorkspace(packageLabel)) {
bazelPackage = bazelWorkspace.getBazelPackage(packageLabel);
}
}
if (bazelPackage == null) {
LOG.debug("{}: ignoring target for unknown package: {}", bazelWorkspace, targetLabel);
continue;
}
if (!result.containsKey(bazelPackage)) {
result.put(bazelPackage, new HashMap<>());
}

var targetName = targetLabel.getTargetName();
result.get(bazelPackage).put(targetName, target);
}
return result;
}

Path workspacePath() {
return root.toPath();
}

}
Loading
Loading