Fix unwanted retention of StarlarkDefinedAspect-s.
When using loaders, they are retained. In particular loader for the inner cache was closed over StarlarkDefinedAspect, which caused a "memory leak".
Refactor, so that only the inner cache is used (making cache non-static).
@@ -90,7 +90,6 @@ public final class StarlarkDefinedAspect implements StarlarkExportable, Starlark
this.applyToGeneratingRules = applyToGeneratingRules;
}
-
public StarlarkCallable getImplementation() {
return implementation;
}
@@ -138,27 +137,21 @@ public final class StarlarkDefinedAspect implements StarlarkExportable, Starlark
private static final ImmutableList<String> ALL_ATTR_ASPECTS = ImmutableList.of("*");
/**
- * The <code>AspectDefinition</code> is a function of the aspect's definition + its parameters, so
- * we can cache that.
- *
- * <p>StarlarkDefinedAspect key: Weak keys use reference comparison, so the values are freed when
- * aspect's definition changes or when it's evicted from the skyframe.
+ * The <code>AspectDefinition</code> is a function of the aspect's parameters, so we can cache
+ * that.
*
- * <p>AspectParameters key: parameters of Starlark aspects are combinatorially limited (only bool,
- * int and enum types). Using strong keys possibly results in a small memory leak. Weak keys don't
- * work because reference equality is used and AspectParameters are created per target.
+ * <p>Parameters of Starlark aspects are combinatorially limited (only bool, int and enum types).
+ * Using strong keys possibly results in a small memory leak. Weak keys don't work because
+ * reference equality is used and AspectParameters are created per target.
*/
- private static final LoadingCache<
- StarlarkDefinedAspect, LoadingCache<AspectParameters, AspectDefinition>>
- definitionCache =
- Caffeine.newBuilder()
- .weakKeys()
- .build(
- starlarkDefinedAspect ->
- Caffeine.newBuilder().build(starlarkDefinedAspect::buildDefinition));
+ private transient LoadingCache<AspectParameters, AspectDefinition> definitionCache =
+ Caffeine.newBuilder().build(this::buildDefinition);
public AspectDefinition getDefinition(AspectParameters aspectParams) {
- return definitionCache.get(this).get(aspectParams);
+ if (definitionCache == null) {
+ definitionCache = Caffeine.newBuilder().build(this::buildDefinition);
+ }
+ return definitionCache.get(aspectParams);
}
private AspectDefinition buildDefinition(AspectParameters aspectParams) {
