gnodet commented on code in PR #1815:
URL: https://github.com/apache/maven-resolver/pull/1815#discussion_r3443247177
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/PremanagedDependency.java:
##########
@@ -92,7 +95,8 @@ public static PremanagedDependency create(
boolean premanagedState) {
DependencyManagement depMngt = depManager != null ?
depManager.manageDependency(dependency) : null;
- int managedBits = 0;
+ EnumMap<DependencyManagement.Subject, Boolean> managedSubjects =
+ new EnumMap<>(DependencyManagement.Subject.class);
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
**Performance:** This `EnumMap` is allocated for **every** dependency during
collection, even when `depMngt` is null (no management applies). Previously
this was `int managedBits = 0` — zero allocation. For a typical enterprise
project, the majority of dependencies are unmanaged.
```suggestion
EnumMap<DependencyManagement.Subject, Boolean> managedSubjects =
null;
```
Then lazily create inside the `if (depMngt != null)` block, e.g.:
```java
if (managedSubjects == null) {
managedSubjects = new EnumMap<>(DependencyManagement.Subject.class);
}
managedSubjects.put(...);
```
`setManagedSubjects(null)` already handles null correctly, so no other
changes needed.
##########
maven-resolver-api/src/main/java/org/eclipse/aether/graph/DefaultDependencyNode.java:
##########
@@ -99,12 +102,25 @@ public DefaultDependencyNode(Artifact artifact) {
* @param node The node to copy, must not be {@code null}.
*/
public DefaultDependencyNode(DependencyNode node) {
- dependency = node.getDependency();
- artifact = node.getArtifact();
- children = new ArrayList<>(0);
+ this.dependency = node.getDependency();
+ this.artifact = node.getArtifact();
+ this.children = new ArrayList<>(0);
setAliases(node.getAliases());
setRequestContext(node.getRequestContext());
- setManagedBits(node.getManagedBits());
+
+ EnumMap<DependencyManagement.Subject, Boolean> managedSubjects =
+ new EnumMap<>(DependencyManagement.Subject.class);
+ for (DependencyManagement.Subject subject :
DependencyManagement.Subject.values()) {
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
**Performance — two issues:**
1. `Subject.values()` allocates a new array each call (Java spec requires a
fresh clone). Cache it:
```java
private static final DependencyManagement.Subject[] SUBJECTS =
DependencyManagement.Subject.values();
```
then use `for (DependencyManagement.Subject subject : SUBJECTS)`.
2. The `EnumMap` is allocated even when the source node has no managed
subjects. Guard the allocation:
```java
EnumMap<DependencyManagement.Subject, Boolean> managedSubjects = null;
for (DependencyManagement.Subject subject : SUBJECTS) {
if (node.isManagedSubject(subject)) {
if (managedSubjects == null) {
managedSubjects = new
EnumMap<>(DependencyManagement.Subject.class);
}
managedSubjects.put(subject, node.isManagedSubjectEnforced(subject));
}
}
setManagedSubjects(managedSubjects);
```
This eliminates both the `isEmpty()` check and the wasted allocation for
unmanaged nodes.
##########
maven-resolver-api/src/main/java/org/eclipse/aether/graph/DefaultDependencyNode.java:
##########
@@ -201,34 +226,83 @@ public void setVersion(Version version) {
this.version = version;
}
+ @Override
public void setScope(String scope) {
if (dependency == null) {
throw new IllegalStateException("node does not have a dependency");
}
dependency = dependency.setScope(scope);
}
+ @Override
public void setOptional(Boolean optional) {
if (dependency == null) {
throw new IllegalStateException("node does not have a dependency");
}
dependency = dependency.setOptional(optional);
}
+ @Override
public int getManagedBits() {
- return managedBits;
+ byte res = 0;
+ if (isManagedSubject(DependencyManagement.Subject.VERSION)) {
+ res |= DependencyNode.MANAGED_VERSION;
+ }
+ if (isManagedSubject(DependencyManagement.Subject.SCOPE)) {
+ res |= DependencyNode.MANAGED_SCOPE;
+ }
+ if (isManagedSubject(DependencyManagement.Subject.OPTIONAL)) {
+ res |= DependencyNode.MANAGED_OPTIONAL;
+ }
+ if (isManagedSubject(DependencyManagement.Subject.PROPERTIES)) {
+ res |= DependencyNode.MANAGED_PROPERTIES;
+ }
+ if (isManagedSubject(DependencyManagement.Subject.EXCLUSIONS)) {
+ res |= DependencyNode.MANAGED_EXCLUSIONS;
+ }
+ return res;
}
- /**
- * Sets a bit field indicating which attributes of this node were subject
to dependency management.
- *
- * @param managedBits The bit field indicating the managed attributes or
{@code 0} if dependency management wasn't
- * applied.
- */
+ @Deprecated
public void setManagedBits(int managedBits) {
- this.managedBits = (byte) (managedBits & 0x1F);
+ EnumMap<DependencyManagement.Subject, Boolean> subjects = new
EnumMap<>(DependencyManagement.Subject.class);
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
**Performance:** Short-circuit for `managedBits == 0` to avoid the wasted
`EnumMap` allocation:
```java
@Deprecated
public void setManagedBits(int managedBits) {
if (managedBits == 0) {
setManagedSubjects(null);
return;
}
EnumMap<DependencyManagement.Subject, Boolean> subjects = new
EnumMap<>(DependencyManagement.Subject.class);
// ... rest unchanged
```
##########
maven-resolver-api/src/main/java/org/eclipse/aether/collection/DependencyManagement.java:
##########
@@ -29,22 +30,46 @@
* @see DependencyManager#manageDependency(org.eclipse.aether.graph.Dependency)
*/
public final class DependencyManagement {
+ /**
+ * Enumeration of manageable attributes, attributes that can be subjected
to dependency management.
+ *
+ * @since 2.0.17
+ */
+ public enum Subject {
+ VERSION,
+ SCOPE,
+ OPTIONAL,
+ EXCLUSIONS,
+ PROPERTIES
+ }
- private String version;
-
- private String scope;
-
- private Boolean optional;
-
- private Collection<Exclusion> exclusions;
-
- private Map<String, String> properties;
+ private final EnumMap<Subject, Object> managedValues;
+ private final EnumMap<Subject, Boolean> managedEnforced;
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
**Performance:** `managedEnforced` only ever stores `true` (if a subject
isn't enforced, it's either absent or `false`). An `EnumSet<Subject>` would be
a better fit — it's backed by a single `long` for enums with ≤64 constants, so
essentially zero overhead vs ~80-100 bytes for the `EnumMap<Subject, Boolean>`.
```java
private final EnumSet<Subject> managedEnforced;
```
Then `setVersion(String version, boolean enforced)` becomes:
```java
if (enforced) {
this.managedEnforced.add(Subject.VERSION);
} else {
this.managedEnforced.remove(Subject.VERSION);
}
```
And `isManagedSubjectEnforced` becomes:
```java
return isManagedSubject(subject) && managedEnforced.contains(subject);
```
This halves the per-`DependencyManagement` allocation cost.
##########
maven-resolver-test-util/src/main/java/org/eclipse/aether/internal/test/util/DependencyGraphParser.java:
##########
@@ -291,16 +293,17 @@ private DependencyNode build(DependencyNode parent,
LineContext ctx, boolean isR
DefaultArtifact artifact = new DefaultArtifact(def.coords,
def.properties);
Dependency dependency = new Dependency(artifact, def.scope,
def.optional);
node = new DefaultDependencyNode(dependency);
- int managedBits = 0;
+ EnumMap<DependencyManagement.Subject, Boolean> managedSubjects =
+ new EnumMap<>(DependencyManagement.Subject.class);
if (def.premanagedScope != null) {
- managedBits |= DependencyNode.MANAGED_SCOPE;
+ managedSubjects.put(DependencyManagement.Subject.SCOPE, true);
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
Managed subjects from the parser are hardcoded as `enforced=true`. This
means graph definition files (`.txt` test resources) cannot express "advised"
management — only programmatic test construction can.
Not blocking, since the existing programmatic tests
(`enforcedScopeNotDerived`, `advisedScopeIsDerived`, etc.) cover the key
scenarios. But if more complex graph-based test scenarios are planned, the
parser format could be extended with a syntax like `scope:test!` (enforced) vs
`scope:test` (advised).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
