Skip to content

Commit

Permalink
Make the cache in PlatformMappingValue strong instead of weak.
Browse files Browse the repository at this point in the history
Clear the cache after each command. The overhead of a weak cache's online cleanup performed during lookups shows up in cpu profiles, and we don't need intra-command cleanup.

PiperOrigin-RevId: 680791811
Change-Id: Ibbd062baac40c4c4311c236cc211269a9a3f87b1
  • Loading branch information
justinhorvitz authored and copybara-github committed Oct 1, 2024
1 parent 0c100ef commit a716fdf
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 1 deletion.
2 changes: 2 additions & 0 deletions src/main/java/com/google/devtools/build/lib/skyframe/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/analysis:inconsistent_null_config_exception",
"//src/main/java/com/google/devtools/build/lib/analysis:invalid_visibility_dependency_exception",
"//src/main/java/com/google/devtools/build/lib/analysis:platform_configuration",
"//src/main/java/com/google/devtools/build/lib/analysis:platform_options",
"//src/main/java/com/google/devtools/build/lib/analysis:required_config_fragments_provider",
"//src/main/java/com/google/devtools/build/lib/analysis:resolved_toolchain_context",
"//src/main/java/com/google/devtools/build/lib/analysis:starlark/starlark_build_settings_details_value",
Expand Down Expand Up @@ -337,6 +338,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2:actiongraph_v2",
"//src/main/java/com/google/devtools/build/lib/skyframe/config",
"//src/main/java/com/google/devtools/build/lib/skyframe/config:exceptions",
"//src/main/java/com/google/devtools/build/lib/skyframe/config:platform_mapping_key",
"//src/main/java/com/google/devtools/build/lib/skyframe/config:sky_functions",
"//src/main/java/com/google/devtools/build/lib/skyframe/rewinding",
"//src/main/java/com/google/devtools/build/lib/skyframe/rewinding:rewindable_graph_inconsistency_receiver",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.ConfiguredTargetValue;
import com.google.devtools.build.lib.analysis.InconsistentNullConfigException;
import com.google.devtools.build.lib.analysis.PlatformOptions;
import com.google.devtools.build.lib.analysis.TargetAndConfiguration;
import com.google.devtools.build.lib.analysis.TargetConfiguredEvent;
import com.google.devtools.build.lib.analysis.TopLevelArtifactContext;
Expand Down Expand Up @@ -219,6 +220,8 @@
import com.google.devtools.build.lib.skyframe.config.ParsedFlagsFunction;
import com.google.devtools.build.lib.skyframe.config.ParsedFlagsValue;
import com.google.devtools.build.lib.skyframe.config.PlatformMappingFunction;
import com.google.devtools.build.lib.skyframe.config.PlatformMappingKey;
import com.google.devtools.build.lib.skyframe.config.PlatformMappingValue;
import com.google.devtools.build.lib.skyframe.rewinding.ActionRewindStrategy;
import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCachingDependenciesProvider;
import com.google.devtools.build.lib.skyframe.toolchains.RegisteredExecutionPlatformsFunction;
Expand Down Expand Up @@ -497,6 +500,8 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {

private SkyfocusState skyfocusState = SkyfocusState.DISABLED;

@Nullable private PlatformMappingKey platformMappingKey;

/**
* Determines the type of hybrid globbing strategy to use when {@link
* #tracksStateForIncrementality()} is {@code true}. See {@link #getGlobbingStrategy()} for more
Expand Down Expand Up @@ -1047,6 +1052,7 @@ public void notifyCommandComplete(ExtendedEventHandler eventHandler) throws Inte
clearSyscallCache();
// So that the supplier object can be GC-ed.
mergedSkyframeAnalysisExecutionSupplier = null;
clearPlatformMappingCache();
}
}

Expand Down Expand Up @@ -2196,6 +2202,16 @@ public void clearSyscallCache() {
syscallCache.clear();
}

private void clearPlatformMappingCache() throws InterruptedException {
if (platformMappingKey == null) {
return;
}
SkyValue platformMappingValue = memoizingEvaluator.getExistingValue(platformMappingKey);
if (platformMappingValue != null) {
((PlatformMappingValue) platformMappingValue).clearMappingCache();
}
}

public void setConflictCheckingModeInThisBuild(
ConflictCheckingMode conflictCheckingModeInThisBuild) {
this.conflictCheckingModeInThisBuild = conflictCheckingModeInThisBuild;
Expand Down Expand Up @@ -2640,6 +2656,8 @@ public WorkspaceInfoFromDiff sync(
String commandName)
throws InterruptedException, AbruptExitException {
getActionEnvFromOptions(options.getOptions(CoreOptions.class));
var platformOptions = options.getOptions(PlatformOptions.class);
platformMappingKey = platformOptions != null ? platformOptions.platformMappingKey : null;
PrecomputedValue.REPO_ENV.set(injectable(), new LinkedHashMap<>(repoEnvOption));
RemoteOptions remoteOptions = options.getOptions(RemoteOptions.class);
setRemoteExecutionEnabled(remoteOptions != null && remoteOptions.isRemoteExecutionEnabled());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public final class PlatformMappingValue implements SkyValue {
this.platformsToFlags = checkNotNull(platformsToFlags);
this.flagsToPlatforms = checkNotNull(flagsToPlatforms);
this.optionsClasses = checkNotNull(optionsClasses);
this.mappingCache = Caffeine.newBuilder().weakKeys().build(this::computeMapping);
this.mappingCache = Caffeine.newBuilder().build(this::computeMapping);
}

/**
Expand Down Expand Up @@ -109,6 +109,11 @@ public BuildConfigurationKey map(BuildOptions original) throws OptionsParsingExc
}
}

/** Clears the mapping cache to save memory. */
public void clearMappingCache() {
mappingCache.invalidateAll();
}

private BuildConfigurationKey computeMapping(BuildOptions originalOptions)
throws OptionsParsingException {
if (originalOptions.hasNoConfig()) {
Expand Down

0 comments on commit a716fdf

Please sign in to comment.