Reducing memory footprint by excluding unused rule attributes (#505)

salesforce · Jul 18, 2024 · 6de8135 · 6de8135
1 parent 75959ba
commit 6de8135
Show file tree

Hide file tree

Showing 10 changed files with 103 additions and 31 deletions.
diff --git a/...e.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelRuleAttributes.java b/...e.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelRuleAttributes.java
@@ -2,6 +2,7 @@
 
 import java.util.List;
 
+import com.salesforce.bazel.sdk.command.querylight.BazelRuleAttribute;
 import com.salesforce.bazel.sdk.command.querylight.Rule;
 
 /**
@@ -19,8 +20,8 @@ public class BazelRuleAttributes {
 
     }
 
-    public Boolean getBoolean(String name) {
-        var attribute = rule.getAttribute(name);
+    public Boolean getBoolean(BazelRuleAttribute name) {
+        var attribute = rule.getAttribute(name.key);
         if (attribute == null) {
             return null;
         }
@@ -35,7 +36,7 @@ public Boolean getBoolean(String name) {
      * @return value of the attribute name if present, otherwise {@link Rule#getName()};
      */
     public String getName() {
-        var name = getString("name");
+        var name = getString(BazelRuleAttribute.NAME);
         if (name != null) {
             return name;
         }
@@ -51,8 +52,8 @@ public String getRuleClass() {
         return rule.ruleClass();
     }
 
-    public String getString(String name) {
-        var attribute = rule.getAttribute(name);
+    public String getString(BazelRuleAttribute name) {
+        var attribute = rule.getAttribute(name.key);
         if (attribute == null) {
             return null;
         }
@@ -63,8 +64,8 @@ public String getString(String name) {
         };
     }
 
-    public List<String> getStringList(String name) {
-        var attribute = rule.getAttribute(name);
+    public List<String> getStringList(BazelRuleAttribute name) {
+        var attribute = rule.getAttribute(name.key);
         if (attribute == null) {
             return null;
         }
@@ -84,7 +85,7 @@ public List<String> getStringList(String name) {
      *         <code>false</code> otherwise
      */
     public boolean hasTag(String tag) {
-        var tags = getStringList("tags");
+        var tags = getStringList(BazelRuleAttribute.TAGS);
         return (tags != null) && tags.contains(tag);
     }
 

diff --git a/...force.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelTargetInfo.java b/...force.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelTargetInfo.java
@@ -27,6 +27,7 @@
 import org.eclipse.core.runtime.IPath;
 import org.eclipse.core.runtime.Status;
 
+import com.salesforce.bazel.sdk.command.querylight.BazelRuleAttribute;
 import com.salesforce.bazel.sdk.command.querylight.Rule;
 import com.salesforce.bazel.sdk.command.querylight.Target;
 import com.salesforce.bazel.sdk.model.BazelLabel;
@@ -154,7 +155,7 @@ public BazelVisibility getVisibility() throws CoreException {
             return cachedVisibility;
         }
 
-        var visibilityValue = getRuleAttributes().getStringList("visibility");
+        var visibilityValue = getRuleAttributes().getStringList(BazelRuleAttribute.VISIBILITY);
         if ((visibilityValue == null) || visibilityValue.isEmpty()) {
             // lookup from package
             return visibility = getBazelTarget().getBazelPackage().getDefaultVisibility();

diff --git a/....core/src/com/salesforce/bazel/eclipse/core/model/discovery/BaseProvisioningStrategy.java b/....core/src/com/salesforce/bazel/eclipse/core/model/discovery/BaseProvisioningStrategy.java
@@ -10,6 +10,15 @@
 import static com.salesforce.bazel.eclipse.core.model.discovery.EclipsePreferencesHelper.convertToPreferences;
 import static com.salesforce.bazel.eclipse.core.model.discovery.JvmConfigurator.VM_TYPE_RUNTIME;
 import static com.salesforce.bazel.eclipse.core.model.discovery.JvmConfigurator.VM_TYPE_TOOLCHAIN;
+import static com.salesforce.bazel.sdk.command.querylight.BazelRuleAttribute.JARS;
+import static com.salesforce.bazel.sdk.command.querylight.BazelRuleAttribute.JAVAC_OPTS;
+import static com.salesforce.bazel.sdk.command.querylight.BazelRuleAttribute.PLUGINS;
+import static com.salesforce.bazel.sdk.command.querylight.BazelRuleAttribute.RESOURCES;
+import static com.salesforce.bazel.sdk.command.querylight.BazelRuleAttribute.RESOURCES_STRIP_PREFIX;
+import static com.salesforce.bazel.sdk.command.querylight.BazelRuleAttribute.SRCS;
+import static com.salesforce.bazel.sdk.command.querylight.BazelRuleAttribute.SRC_JAR;
+import static com.salesforce.bazel.sdk.command.querylight.BazelRuleAttribute.STRIP_PREFIX;
+import static com.salesforce.bazel.sdk.command.querylight.BazelRuleAttribute.TEST_ONLY;
 import static java.lang.Boolean.TRUE;
 import static java.lang.String.format;
 import static java.util.Objects.requireNonNull;
@@ -156,11 +165,11 @@ private void addInfoFromTarget(JavaProjectInfo javaInfo, BazelTarget bazelTarget
 
         var attributes = bazelTarget.getRuleAttributes();
 
-        var testonly = attributes.getBoolean("testonly");
+        var testonly = attributes.getBoolean(TEST_ONLY);
         isTestTarget = isTestTarget || ((testonly != null) && testonly.booleanValue());
 
         var nowarn = false;
-        var javacOpts = attributes.getStringList("javacopts");
+        var javacOpts = attributes.getStringList(JAVAC_OPTS);
         if (javacOpts != null) {
             for (String javacOpt : javacOpts) {
                 javaInfo.addJavacOpt(javacOpt);
@@ -171,7 +180,7 @@ private void addInfoFromTarget(JavaProjectInfo javaInfo, BazelTarget bazelTarget
         }
         var settings = nowarn ? new EntrySettings(nowarn) : EntrySettings.DEFAULT_SETTINGS;
 
-        var srcs = attributes.getStringList("srcs");
+        var srcs = attributes.getStringList(SRCS);
         if (srcs != null) {
             for (String src : srcs) {
                 if (isTestTarget) {
@@ -182,9 +191,9 @@ private void addInfoFromTarget(JavaProjectInfo javaInfo, BazelTarget bazelTarget
             }
         }
 
-        var resources = attributes.getStringList("resources");
+        var resources = attributes.getStringList(RESOURCES);
         if (resources != null) {
-            var resourceStripPrefix = attributes.getString("resource_strip_prefix");
+            var resourceStripPrefix = attributes.getString(RESOURCES_STRIP_PREFIX);
             for (String resource : resources) {
                 if (isTestTarget) {
                     javaInfo.addTestResource(resource, resourceStripPrefix);
@@ -194,17 +203,17 @@ private void addInfoFromTarget(JavaProjectInfo javaInfo, BazelTarget bazelTarget
             }
         }
 
-        var pluginDeps = attributes.getStringList("plugins");
+        var pluginDeps = attributes.getStringList(PLUGINS);
         if (pluginDeps != null) {
             for (String dep : pluginDeps) {
                 javaInfo.addPluginDep(dep);
 
             }
         }
 
-        var jars = attributes.getStringList("jars");
+        var jars = attributes.getStringList(JARS);
         if (jars != null) {
-            var srcJar = attributes.getString("srcjar");
+            var srcJar = attributes.getString(SRC_JAR);
             for (String jar : jars) {
                 // java_import is generally used to make classes and resources available on the classpath
                 // lets check if we can translate this to resources in the same Bazel package
@@ -363,10 +372,10 @@ private void addResourcesFromRulesPkgRules(JavaProjectInfo javaInfo, BazelTarget
         }
 
         // inspect srcs
-        var srcs = rulesPkgTarget.getRuleAttributes().getStringList("srcs");
+        var srcs = rulesPkgTarget.getRuleAttributes().getStringList(SRCS);
         if (srcs != null) {
             // the strip_prefix in rules_pkg is relative to the package, need to make it absolute
-            var resourceStripPrefix = rulesPkgTarget.getRuleAttributes().getString("strip_prefix");
+            var resourceStripPrefix = rulesPkgTarget.getRuleAttributes().getString(STRIP_PREFIX);
             if ((resourceStripPrefix != null) && !resourceStripPrefix.isEmpty()) {
                 resourceStripPrefix = rulesPkgTarget.getBazelPackage()
                         .getWorkspaceRelativePath()
@@ -1309,7 +1318,7 @@ private boolean hasTestSources(BazelTarget bazelTarget) throws CoreException {
         }
 
         var attributes = bazelTarget.getRuleAttributes();
-        var srcs = attributes.getStringList("srcs");
+        var srcs = attributes.getStringList(SRCS);
         if (srcs != null) {
             for (String src : srcs) {
                 if (src.contains(BazelLabel.BAZEL_COLON)) {

diff --git a/....core/src/com/salesforce/bazel/eclipse/core/model/discovery/JavaAspectsClasspathInfo.java b/....core/src/com/salesforce/bazel/eclipse/core/model/discovery/JavaAspectsClasspathInfo.java
@@ -66,6 +66,7 @@
 import com.salesforce.bazel.eclipse.core.model.discovery.classpath.AccessRule;
 import com.salesforce.bazel.eclipse.core.model.discovery.classpath.ClasspathEntry;
 import com.salesforce.bazel.sdk.command.BazelBuildWithIntelliJAspectsCommand;
+import com.salesforce.bazel.sdk.command.querylight.BazelRuleAttribute;
 
 /**
  * Holds information for computing Java classpath configuration of a target or a package.
@@ -281,7 +282,7 @@ public IStatus addTarget(BazelTarget bazelTarget) throws CoreException {
         }
 
         // collect exports
-        var exports = bazelTarget.getRuleAttributes().getStringList("exports");
+        var exports = bazelTarget.getRuleAttributes().getStringList(BazelRuleAttribute.EXPORTS);
         if (exports != null) {
             for (String export : exports) {
                 this.exports.add(Label.create(export));
@@ -411,7 +412,7 @@ private BazelWorkspace findExternalWorkspace(Label label) throws CoreException {
         if (externalRepository != null) {
             switch (externalRepository.getRuleClass()) {
                 case "local_repository": {
-                    var pathAttribute = externalRepository.getString("path");
+                    var pathAttribute = externalRepository.getString(BazelRuleAttribute.PATH);
                     if (pathAttribute != null) {
                         var path = forPosix(pathAttribute);
                         if (!path.isAbsolute()) {

diff --git a/...sforce/bazel/eclipse/core/model/discovery/classpath/libs/GeneratedLibrariesDiscovery.java b/...sforce/bazel/eclipse/core/model/discovery/classpath/libs/GeneratedLibrariesDiscovery.java
@@ -42,6 +42,7 @@
 import com.salesforce.bazel.eclipse.core.model.discovery.classpath.ClasspathEntry;
 import com.salesforce.bazel.eclipse.core.util.jar.SourceJarFinder;
 import com.salesforce.bazel.sdk.command.BazelQueryForTargetProtoCommand;
+import com.salesforce.bazel.sdk.command.querylight.BazelRuleAttribute;
 import com.salesforce.bazel.sdk.command.querylight.GeneratedFile;
 import com.salesforce.bazel.sdk.command.querylight.Target;
 import com.salesforce.bazel.sdk.model.BazelLabel;
@@ -92,8 +93,7 @@ private void queryForGeneratedJars(Set<ClasspathEntry> result) throws CoreExcept
         Map<String, List<String>> jarsByGeneratingRuleLabel = generatedJarTargets.stream()
                 .map(Target::generatedFile)
                 .filter(f -> !f.name().endsWith("_deploy.jar")) // ignore deploy jars (they just duplicate class files)
-                .collect(
-                    groupingBy(GeneratedFile::generatingRule, mapping(GeneratedFile::name, toList())));
+                .collect(groupingBy(GeneratedFile::generatingRule, mapping(GeneratedFile::name, toList())));
 
         // ensure all packages are open
         bazelWorkspace.open(
@@ -145,7 +145,8 @@ private void queryForGeneratedJars(Set<ClasspathEntry> result) throws CoreExcept
                 LOG.debug("No jars left in target '{}", bazelTarget);
                 continue;
             }
-            var testOnly = Optional.ofNullable(bazelTarget.getRuleAttributes().getBoolean("testonly"));
+            var testOnly =
+                    Optional.ofNullable(bazelTarget.getRuleAttributes().getBoolean(BazelRuleAttribute.TEST_ONLY));
             var origin = bazelTarget.getLabel().toPrimitive();
 
             collectJarsAsClasspathEntries(classpathJars, srcJar, testOnly, origin, result);

diff --git a/...zel.eclipse.ui/src/com/salesforce/bazel/eclipse/ui/jdt/FindClassResolutionsOperation.java b/...zel.eclipse.ui/src/com/salesforce/bazel/eclipse/ui/jdt/FindClassResolutionsOperation.java
@@ -41,6 +41,7 @@
 import com.salesforce.bazel.eclipse.core.model.discovery.classpath.util.TypeLocator;
 import com.salesforce.bazel.eclipse.ui.jdt.JavaResolutionFactory.ProposalType;
 import com.salesforce.bazel.eclipse.ui.utils.JavaSearchUtil;
+import com.salesforce.bazel.sdk.command.querylight.BazelRuleAttribute;
 
 /**
  * This Operation is used to find possible resolutions to an unresolved class reference in a Bazel project.
@@ -179,7 +180,8 @@ public void acceptSearchMatch(SearchMatch aMatch) throws CoreException {
             }
             new SearchEngine().search(
                 typeOrPackagePattern,
-                new SearchParticipant[] { SearchEngine.getDefaultSearchParticipant() },
+                new SearchParticipant[] {
+                        SearchEngine.getDefaultSearchParticipant() },
                 searchScope,
                 requestor,
                 subMonitor.split(1));
@@ -209,7 +211,7 @@ public void acceptSearchMatch(SearchMatch aMatch) throws CoreException {
 
     private void removeAllTargetDepsFromMap(final Map<Label, ClasspathEntry> bazelInfos, BazelTarget bazelTarget)
             throws CoreException {
-        var deps = bazelTarget.getRuleAttributes().getStringList("deps");
+        var deps = bazelTarget.getRuleAttributes().getStringList(BazelRuleAttribute.DEPS);
         if (deps != null) {
             for (String dep : deps) {
                 bazelInfos.remove(Label.create(dep));

diff --git a/...esforce.bazel.sdk/src/com/salesforce/bazel/sdk/command/querylight/BazelRuleAttribute.java b/...esforce.bazel.sdk/src/com/salesforce/bazel/sdk/command/querylight/BazelRuleAttribute.java
@@ -0,0 +1,49 @@
+/*-
+ * Copyright (c) 2024 Salesforce and others.
+ *
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *      Salesforce - adapted from M2E, JDT or other Eclipse project
+ */
+package com.salesforce.bazel.sdk.command.querylight;
+
+import java.util.Arrays;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+/**
+ * Rule attribute names we depend on during sync
+ */
+public enum BazelRuleAttribute {
+
+    SRCS("srcs"),
+    EXPORTS("exports"),
+    TEST_ONLY("testonly"),
+    JAVAC_OPTS("javacopts"),
+    RESOURCES("resources"),
+    RESOURCES_STRIP_PREFIX("resource_strip_prefix"),
+    STRIP_PREFIX("strip_prefix"),
+    PLUGINS("plugins"),
+    JARS("jars"),
+    SRC_JAR("srcjar"),
+    TAGS("tags"),
+    NAME("name"),
+    VISIBILITY("visibility"),
+    PATH("path"),
+    DEPS("deps");
+
+    public static final Set<String> KNOWN_ATTRIBUTES =
+            Arrays.stream(BazelRuleAttribute.values()).map(attr -> attr.key).collect(Collectors.toSet());
+
+    public final String key;
+
+    BazelRuleAttribute(String key) {
+        this.key = key;
+    }
+}
diff --git a/bundles/com.salesforce.bazel.sdk/src/com/salesforce/bazel/sdk/command/querylight/Rule.java b/bundles/com.salesforce.bazel.sdk/src/com/salesforce/bazel/sdk/command/querylight/Rule.java
@@ -31,7 +31,8 @@ public record Rule(String name, String ruleClass, List<String> ruleOutputList, M
                 rule.getAttributeList().stream().collect(HashMap::new, (map, attribute) -> {
                     // multiple attributes with the same name are not expected but can happen (https://github.com/bazelbuild/bazel/issues/20918)
                     // we therefore store the first occurrence of an attribute
-                    if (!map.containsKey(attribute.getName())) {
+                    if (BazelRuleAttribute.KNOWN_ATTRIBUTES.contains(attribute.getName())
+                            && !map.containsKey(attribute.getName())) {
                         map.put(attribute.getName(), new Attribute(attribute));
                     }
                 }, HashMap::putAll));

diff --git a/tests/com.salesforce.bazel.eclipse.core.tests/META-INF/MANIFEST.MF b/tests/com.salesforce.bazel.eclipse.core.tests/META-INF/MANIFEST.MF
@@ -12,4 +12,5 @@ Import-Package: org.hamcrest;version="2.2.0",
  org.junit.jupiter.api;version="5.9.2",
  org.junit.jupiter.api.extension;version="5.9.2",
  org.junit.jupiter.api.io;version="5.9.2"
-Require-Bundle: testdata;bundle-version="1.0.0"
+Require-Bundle: testdata;bundle-version="1.0.0",
+ com.salesforce.bazel.sdk
diff --git a/...rc/com/salesforce/bazel/eclipse/core/tests/it/Workspace001Test_BasicModelAssumptions.java b/...rc/com/salesforce/bazel/eclipse/core/tests/it/Workspace001Test_BasicModelAssumptions.java
@@ -19,6 +19,7 @@
 import com.salesforce.bazel.eclipse.core.model.BazelPackage;
 import com.salesforce.bazel.eclipse.core.model.BazelWorkspace;
 import com.salesforce.bazel.sdk.BazelVersion;
+import com.salesforce.bazel.sdk.command.querylight.BazelRuleAttribute;
 import com.salesforce.bazel.sdk.model.BazelLabel;
 
 import testdata.SharedTestData;
@@ -84,7 +85,12 @@ void test0010_module1() throws Exception {
         var ruleAttributes = module1.getRuleAttributes();
         assertNotNull(ruleAttributes);
 
-        assertThat(ruleAttributes.getStringList("deps"), hasItems("//module2:module2", "//module3:module3",
-            "@com_google_guava//jar:jar", "//module1:mybuilder_sources"));
+        assertThat(
+            ruleAttributes.getStringList(BazelRuleAttribute.DEPS),
+            hasItems(
+                "//module2:module2",
+                "//module3:module3",
+                "@com_google_guava//jar:jar",
+                "//module1:mybuilder_sources"));
     }
 }