diff options
author | Jon Brandvein <brandjon@google.com> | 2016-12-19 23:09:30 +0000 |
---|---|---|
committer | Yue Gan <yueg@google.com> | 2016-12-20 06:13:31 +0000 |
commit | db9d6f15ac69ded498652806924e85a04a59d65b (patch) | |
tree | e4b74b8cc0df39f529c5148cae7356a612898499 /src/main/java | |
parent | 8e1cd5d9ff931d07f2724e1f08e2cd8f9442fdf4 (diff) |
Cleanup FileWriteAction and add a flag that will guard transparent compression
This clarifies documentation, renames or rearranges constructors, and defines a BuildConfiguration option that will be made to control transparent compression in a follow-up CL. The follow-up updates call sites to use the new create() factory method.
--
PiperOrigin-RevId: 142491333
MOS_MIGRATED_REVID=142491333
Diffstat (limited to 'src/main/java')
-rw-r--r-- | src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteAction.java | 164 | ||||
-rw-r--r-- | src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java | 28 |
2 files changed, 142 insertions, 50 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteAction.java index 7b0907a570..b6234ccff6 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteAction.java @@ -16,6 +16,7 @@ package com.google.devtools.build.lib.analysis.actions; import static java.nio.charset.StandardCharsets.UTF_8; +import com.google.common.annotations.VisibleForTesting; import com.google.devtools.build.lib.actions.ActionExecutionContext; import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; @@ -32,59 +33,131 @@ import java.util.zip.GZIPInputStream; import java.util.zip.GZIPOutputStream; /** - * Action to write to a file. - * <p>TODO(bazel-team): Choose a better name to distinguish this class from - * {@link BinaryFileWriteAction}. + * Action to write a file whose contents are known at analysis time. + * + * <p>The output is always UTF-8 encoded. + * + * <p>TODO(bazel-team): Choose a better name to distinguish this class from {@link + * BinaryFileWriteAction}. */ @Immutable // if fileContents is immutable public final class FileWriteAction extends AbstractFileWriteAction { + /** Whether or not transparent compression is possible. */ + public static enum Compression { + /** No compression, ever. */ + DISALLOW, + /** May compress. */ + ALLOW; + + /** Maps true/false to allow/disallow respectively. */ + public static Compression fromBoolean(boolean allow) { + return allow ? ALLOW : DISALLOW; + } + } + private static final String GUID = "332877c7-ca9f-4731-b387-54f620408522"; /** - * We keep it as a CharSequence for memory-efficiency reasons. The toString() - * method of the object represents the content of the file. + * The contents may be lazily computed or compressed. * - * <p>For example, this allows us to keep a {@code List<Artifact>} wrapped - * in a {@code LazyString} instead of the string representation of the concatenation. - * This saves memory because the Artifacts are shared objects but the - * resulting String is not. + * <p>If the object representing the contents is a {@code String}, its length is greater than + * {@code COMPRESS_CHARS_THRESHOLD}, and compression is enabled, then the gzipped bytestream of + * the contents will be stored in place of the string itself. This compression is transparent and + * does not affect the output file. + * + * <p>Otherwise, if the object represents a lazy computation, it will not be forced until {@link + * #getFileContents()} is called. An example where this may come in handy is if the contents are + * the concatenation of the string representations of a series of artifacts. Then the client code + * can wrap a {@code List<Artifact>} in a {@code LazyString}, which saves memory since the + * artifacts are shared objects whereas a string is not. */ private final CharSequence fileContents; + /** Minimum length (in chars) for content to be eligible for compression. */ + private static final int COMPRESS_CHARS_THRESHOLD = 256; + /** - * Creates a new FileWriteAction instance without inputs using UTF8 encoding. + * Creates a new FileWriteAction instance without inputs. * - * @param owner the action owner. - * @param output the Artifact that will be created by executing this Action. - * @param fileContents the contents to be written to the file. - * @param makeExecutable iff true will change the output file to be - * executable. + * @param owner the action owner + * @param output the Artifact that will be created by executing this Action + * @param fileContents the contents to be written to the file + * @param makeExecutable whether the output file is made executable */ - public FileWriteAction(ActionOwner owner, Artifact output, CharSequence fileContents, - boolean makeExecutable) { - this(owner, Artifact.NO_ARTIFACTS, output, fileContents, makeExecutable); + public FileWriteAction( + ActionOwner owner, Artifact output, CharSequence fileContents, boolean makeExecutable) { + this(owner, Artifact.NO_ARTIFACTS, output, fileContents, makeExecutable, Compression.DISALLOW); } /** - * Creates a new FileWriteAction instance using UTF8 encoding. + * Creates a new FileWriteAction instance with inputs and with direct control over whether + * compression may occur. + * + * <p>The contents of the inputs are not actually read, but including them will ensure that their + * generating actions are executed if this action is. * - * @param owner the action owner. + * @param owner the action owner * @param inputs the Artifacts that this Action depends on - * @param output the Artifact that will be created by executing this Action. - * @param fileContents the contents to be written to the file. - * @param makeExecutable iff true will change the output file to be - * executable. + * @param output the Artifact that will be created by executing this Action + * @param fileContents the contents to be written to the file + * @param makeExecutable whether the output file is made executable + * @param allowCompression whether (transparent) compression is enabled */ - public FileWriteAction(ActionOwner owner, Collection<Artifact> inputs, - Artifact output, CharSequence fileContents, boolean makeExecutable) { + public FileWriteAction( + ActionOwner owner, + Collection<Artifact> inputs, + Artifact output, + CharSequence fileContents, + boolean makeExecutable, + Compression allowCompression) { super(owner, inputs, output, makeExecutable); - if (fileContents instanceof String && fileContents.length() > 256) { + if (allowCompression == Compression.ALLOW + && fileContents instanceof String + && fileContents.length() > COMPRESS_CHARS_THRESHOLD) { fileContents = new CompressedString((String) fileContents); } this.fileContents = fileContents; } + /** + * Creates a new FileWriteAction instance with inputs and empty content. + * + * <p>This is useful for producing an artifact that, if built, will ensure that the generating + * actions for its inputs are run. + * + * @param owner the action owner + * @param inputs the Artifacts that this Action depends on + * @param output the Artifact that will be created by executing this Action + */ + public static FileWriteAction createEmpty( + ActionOwner owner, Collection<Artifact> inputs, Artifact output) { + return new FileWriteAction(owner, inputs, output, "", false, Compression.DISALLOW); + } + + /** + * Creates a new FileWriteAction instance. + * + * <p>There are no inputs. Transparent compression is controlled by the {@code + * --experimental_transparent_compression} flag. No reference to the {@link RuleContext} will be + * maintained. + * + * @param context the rule context + * @param output the Artifact that will be created by executing this Action + * @param fileContents the contents to be written to the file + * @param makeExecutable whether the output file is made executable + */ + public static FileWriteAction create( + RuleContext context, Artifact output, CharSequence fileContents, boolean makeExecutable) { + return new FileWriteAction( + context.getActionOwner(), + Artifact.NO_ARTIFACTS, + output, + fileContents, + makeExecutable, + context.getConfiguration().transparentCompression()); + } + private static final class CompressedString extends LazyString { final byte[] bytes; final int uncompressedSize; @@ -125,20 +198,17 @@ public final class FileWriteAction extends AbstractFileWriteAction { } } + @VisibleForTesting + boolean usesCompression() { + return fileContents instanceof CompressedString; + } + /** - * Creates a new FileWriteAction instance using UTF8 encoding. + * Returns the string contents to be written. * - * @param owner the action owner. - * @param inputs the Artifacts that this Action depends on - * @param output the Artifact that will be created by executing this Action. - * @param makeExecutable iff true will change the output file to be - * executable. + * <p>Note that if the string is lazily computed or compressed, calling this method will force its + * computation or decompression. No attempt is made by FileWriteAction to cache the result. */ - protected FileWriteAction(ActionOwner owner, Collection<Artifact> inputs, - Artifact output, boolean makeExecutable) { - this(owner, inputs, output, "", makeExecutable); - } - public String getFileContents() { return fileContents.toString(); } @@ -176,17 +246,17 @@ public final class FileWriteAction extends AbstractFileWriteAction { } /** - * Creates a FileWriteAction to write contents to the resulting artifact - * fileName in the genfiles root underneath the package path. + * Creates a FileWriteAction to write contents to the resulting artifact fileName in the genfiles + * root underneath the package path. * - * @param ruleContext the ruleContext that will own the action of creating this file. - * @param fileName name of the file to create. - * @param contents data to write to file. - * @param executable flags that file should be marked executable. - * @return Artifact describing the file to create. + * @param ruleContext the ruleContext that will own the action of creating this file + * @param fileName name of the file to create + * @param contents data to write to file + * @param executable flags that file should be marked executable + * @return Artifact describing the file to create */ - public static Artifact createFile(RuleContext ruleContext, - String fileName, CharSequence contents, boolean executable) { + public static Artifact createFile( + RuleContext ruleContext, String fileName, CharSequence contents, boolean executable) { Artifact scriptFileArtifact = ruleContext.getPackageRelativeArtifact( fileName, ruleContext.getConfiguration().getGenfilesDirectory( ruleContext.getRule().getRepository())); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java index 05da5c83cb..ad6104061e 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java @@ -37,6 +37,7 @@ import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; import com.google.devtools.build.lib.analysis.Dependency; import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.analysis.actions.FileWriteAction; import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection.Transitions; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; @@ -816,6 +817,19 @@ public final class BuildConfiguration { help = "Use action_listener to attach an extra_action to existing build actions.") public List<Label> actionListeners; + // TODO(bazel-team): Either remove this flag once transparent compression is shown to not + // noticeably affect running time, or keep this flag and move it into a new configuration + // fragment. + @Option( + name = "experimental_transparent_compression", + defaultValue = "true", + category = "undocumented", + help = + "Enables gzip compression for the contents of FileWriteActions, which reduces " + + "memory usage in the analysis phase at the expense of additional time overhead." + ) + public boolean transparentCompression; + @Option(name = "is host configuration", defaultValue = "false", category = "undocumented", @@ -2279,9 +2293,17 @@ public final class BuildConfiguration { } /** - * Returns whether we should use dynamically instantiated build configurations - * vs. static configurations (e.g. predefined in - * {@link com.google.devtools.build.lib.analysis.ConfigurationCollectionFactory}). + * Returns whether FileWriteAction may transparently compress its contents in the analysis phase + * to save memory. Semantics are not affected. + */ + public FileWriteAction.Compression transparentCompression() { + return FileWriteAction.Compression.fromBoolean(options.transparentCompression); + } + + /** + * Returns whether we should use dynamically instantiated build configurations vs. static + * configurations (e.g. predefined in {@link + * com.google.devtools.build.lib.analysis.ConfigurationCollectionFactory}). */ public boolean useDynamicConfigurations() { return options.useDynamicConfigurations != Options.DynamicConfigsMode.OFF; |