BEP: Extend infrastructure to allow reporting artifacts only once

Extend the functionality of the BuildEventStreamer to report those parts
of NestedSets of Artifacts not reported earlier. In this way, duplicate
reporting can be avoided, without the events themselves having to know
which artifacts are known already.

Change-Id: Ia959c28c440301860eac57ea5d9a712c0d49ebdf
PiperOrigin-RevId: 152497672

10 files changed, 237 insertions, 18 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/EventReportingArtifacts.java b/src/main/java/com/google/devtools/build/lib/actions/EventReportingArtifacts.java
new file mode 100644
index 0000000000..a81fc00f50
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/actions/EventReportingArtifacts.java
@@ -0,0 +1,25 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.actions;
+
+import com.google.devtools.build.lib.buildeventstream.BuildEvent;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
+import java.util.Collection;
+
+/** Interface for {@link BuildEvent}s reporting artifacts as named sets */
+public interface EventReportingArtifacts extends BuildEvent {
+  /** The sets of artifacts this build event asumes already known in the build event stream. */
+  Collection<NestedSet<Artifact>> reportedArtifacts();
+}
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/ArtifactGroupNamer.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/ArtifactGroupNamer.java
new file mode 100644
index 0000000000..ede4767e41
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/ArtifactGroupNamer.java
@@ -0,0 +1,26 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.buildeventstream;
+
+
+/** Interface for conversion of paths to URIs. */
+public interface ArtifactGroupNamer {
+  /**
+   * Return the name of a declared group of artifacts, identified by the identifier of their {@link
+   * NestedSetView}. A {@link BuildEvent} should only assume that this function is defined if the
+   * corresponding {@link NestedSet<Artifact>} is declared via the {@link EventReportingArtifacts}
+   * interface. On undefined positions, the value null is returned.
+   */
+  BuildEventStreamProtos.BuildEventId.NamedSetOfFilesId apply(Object id);
+}
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventConverters.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventConverters.java
index 1943c1f2cf..2ade08695d 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventConverters.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventConverters.java
@@ -24,4 +24,10 @@ public interface BuildEventConverters {
    * be reported in the event.
    */
   PathConverter pathConverter();
+
+  /**
+   * Return the {@link ArtifactGroupNamer} that can be used to refer to a {@link
+   * NestedSet<Artifact>} declared via the {@link EventReportingArtifacts} interface.
+   */
+  ArtifactGroupNamer artifactGroupNamer();
 }
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventId.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventId.java
index 7fa1bf1c60..33151e9837 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventId.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventId.java
@@ -156,6 +156,13 @@ public final class BuildEventId implements Serializable {
             .build());
   }
 
+  public static BuildEventId fromArtifactGroupName(String name) {
+    BuildEventStreamProtos.BuildEventId.NamedSetOfFilesId namedSetId =
+        BuildEventStreamProtos.BuildEventId.NamedSetOfFilesId.newBuilder().setId(name).build();
+    return new BuildEventId(
+        BuildEventStreamProtos.BuildEventId.newBuilder().setNamedSet(namedSetId).build());
+  }
+
   public static BuildEventId testResult(Label target, Integer run, Integer shard, Integer attempt) {
     BuildEventStreamProtos.BuildEventId.TestResultId resultId =
         BuildEventStreamProtos.BuildEventId.TestResultId.newBuilder()
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventTransport.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventTransport.java
index 318282fc27..dba5a123d2 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventTransport.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventTransport.java
@@ -38,7 +38,7 @@ public interface BuildEventTransport {
    *
    * @param event the event to sendBuildEvent.
    */
-  void sendBuildEvent(BuildEvent event);
+  void sendBuildEvent(BuildEvent event, ArtifactGroupNamer namer);
 
   /**
    * Initiates a close. Callers may listen to the returned future to be notified when the close
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/BinaryFormatFileTransport.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/BinaryFormatFileTransport.java
index ccfa14c0b2..9de20b7175 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/BinaryFormatFileTransport.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/BinaryFormatFileTransport.java
@@ -16,7 +16,9 @@ package com.google.devtools.build.lib.buildeventstream.transports;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 
+import com.google.devtools.build.lib.buildeventstream.ArtifactGroupNamer;
 import com.google.devtools.build.lib.buildeventstream.BuildEvent;
+import com.google.devtools.build.lib.buildeventstream.BuildEventConverters;
 import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos;
 import com.google.devtools.build.lib.buildeventstream.BuildEventTransport;
 import com.google.devtools.build.lib.buildeventstream.PathConverter;
@@ -34,13 +36,26 @@ public final class BinaryFormatFileTransport extends FileTransport {
   private static final Logger log = Logger.getLogger(BinaryFormatFileTransport.class.getName());
 
   private static final int MAX_VARINT_BYTES = 9;
+  private final PathConverter pathConverter;
 
   BinaryFormatFileTransport(String path, PathConverter pathConverter) {
-    super(path, pathConverter);
+    super(path);
+    this.pathConverter = pathConverter;
   }
 
   @Override
-  public void sendBuildEvent(BuildEvent event) {
+  public synchronized void sendBuildEvent(BuildEvent event, final ArtifactGroupNamer namer) {
+    BuildEventConverters converters =
+        new BuildEventConverters() {
+          @Override
+          public PathConverter pathConverter() {
+            return pathConverter;
+          }
+          @Override
+          public ArtifactGroupNamer artifactGroupNamer() {
+            return namer;
+          }
+        };
     checkNotNull(event);
     BuildEventStreamProtos.BuildEvent protoEvent = event.asStreamProto(converters);
 
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/FileTransport.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/FileTransport.java
index e691df1819..6744df3ef6 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/FileTransport.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/FileTransport.java
@@ -20,9 +20,7 @@ import static com.google.common.base.Preconditions.checkState;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.util.concurrent.SettableFuture;
 import com.google.devtools.build.lib.buildeventstream.BuildEvent;
-import com.google.devtools.build.lib.buildeventstream.BuildEventConverters;
 import com.google.devtools.build.lib.buildeventstream.BuildEventTransport;
-import com.google.devtools.build.lib.buildeventstream.PathConverter;
 import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.nio.channels.AsynchronousFileChannel;
@@ -55,7 +53,6 @@ abstract class FileTransport implements BuildEventTransport {
   @VisibleForTesting
   final AsynchronousFileChannel ch;
   private final WriteCompletionHandler completionHandler = new WriteCompletionHandler();
-  protected final BuildEventConverters converters;
   // The offset in the file to begin the next write at.
   private long writeOffset;
   // Number of writes that haven't completed yet.
@@ -63,19 +60,13 @@ abstract class FileTransport implements BuildEventTransport {
   // The future returned by close()
   private SettableFuture<Void> closeFuture;
 
-  FileTransport(String path, final PathConverter pathConverter) {
+  FileTransport(String path) {
     try {
       ch = AsynchronousFileChannel.open(Paths.get(path), StandardOpenOption.CREATE,
           StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.WRITE);
     } catch (IOException e) {
       throw new RuntimeException(e);
     }
-    this.converters = new BuildEventConverters() {
-      @Override
-      public PathConverter pathConverter() {
-        return pathConverter;
-      }
-    };
   }
 
   synchronized void writeData(byte[] data) {
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/TextFormatFileTransport.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/TextFormatFileTransport.java
index e331c0c573..cf892955a7 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/TextFormatFileTransport.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/TextFormatFileTransport.java
@@ -14,7 +14,9 @@
 
 package com.google.devtools.build.lib.buildeventstream.transports;
 
+import com.google.devtools.build.lib.buildeventstream.ArtifactGroupNamer;
 import com.google.devtools.build.lib.buildeventstream.BuildEvent;
+import com.google.devtools.build.lib.buildeventstream.BuildEventConverters;
 import com.google.devtools.build.lib.buildeventstream.BuildEventTransport;
 import com.google.devtools.build.lib.buildeventstream.PathConverter;
 import com.google.protobuf.TextFormat;
@@ -28,15 +30,29 @@ import java.io.IOException;
  */
 public final class TextFormatFileTransport extends FileTransport {
 
+  private final PathConverter pathConverter;
+
   TextFormatFileTransport(String path, PathConverter pathConverter) throws IOException {
-    super(path, pathConverter);
+    super(path);
+    this.pathConverter = pathConverter;
   }
 
   @Override
-  public void sendBuildEvent(BuildEvent event) {
+  public synchronized void sendBuildEvent(BuildEvent event, final ArtifactGroupNamer namer) {
+    BuildEventConverters converters =
+        new BuildEventConverters() {
+          @Override
+          public PathConverter pathConverter() {
+            return pathConverter;
+          }
+
+          @Override
+          public ArtifactGroupNamer artifactGroupNamer() {
+            return namer;
+          }
+        };
     String protoTextRepresentation = TextFormat.printToString(event.asStreamProto(converters));
     String line = "event {\n" + protoTextRepresentation + "}\n\n";
-
     writeData(line.getBytes());
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamer.java b/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamer.java
index 697cd155a2..63583149fc 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamer.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamer.java
@@ -21,23 +21,31 @@ import com.google.common.collect.Multimap;
 import com.google.common.collect.Sets;
 import com.google.common.eventbus.Subscribe;
 import com.google.devtools.build.lib.actions.ActionExecutedEvent;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.EventReportingArtifacts;
 import com.google.devtools.build.lib.analysis.NoBuildEvent;
 import com.google.devtools.build.lib.buildeventstream.AbortedEvent;
+import com.google.devtools.build.lib.buildeventstream.ArtifactGroupNamer;
 import com.google.devtools.build.lib.buildeventstream.BuildEvent;
 import com.google.devtools.build.lib.buildeventstream.BuildEventId;
 import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.Aborted.AbortReason;
+import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildEventId.NamedSetOfFilesId;
 import com.google.devtools.build.lib.buildeventstream.BuildEventTransport;
 import com.google.devtools.build.lib.buildeventstream.BuildEventWithOrderConstraint;
 import com.google.devtools.build.lib.buildeventstream.ProgressEvent;
 import com.google.devtools.build.lib.buildtool.buildevent.BuildCompleteEvent;
 import com.google.devtools.build.lib.buildtool.buildevent.BuildInterruptedEvent;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
+import com.google.devtools.build.lib.collect.nestedset.NestedSetView;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.rules.extra.ExtraAction;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.Future;
 import java.util.logging.Logger;
@@ -50,9 +58,41 @@ public class BuildEventStreamer implements EventHandler {
   private final Set<BuildEventId> postedEvents = new HashSet<>();
   private final Multimap<BuildEventId, BuildEvent> pendingEvents = HashMultimap.create();
   private int progressCount;
+  private final CountingArtifactGroupNamer artifactGroupNamer = new CountingArtifactGroupNamer();
   private AbortReason abortReason = AbortReason.UNKNOWN;
   private static final Logger log = Logger.getLogger(BuildEventStreamer.class.getName());
 
+  private static class CountingArtifactGroupNamer implements ArtifactGroupNamer {
+    private final Map<Object, Long> reportedArtifactNames = new HashMap<>();
+    private long nextArtifactName;
+
+    @Override
+    public NamedSetOfFilesId apply(Object id) {
+      Long name;
+      synchronized (this) {
+        name = reportedArtifactNames.get(id);
+      }
+      if (name == null) {
+        return null;
+      }
+      return NamedSetOfFilesId.newBuilder().setId(name.toString()).build();
+    }
+
+    /**
+     * If the {@link NestedSetView} has no name already, return a new name for it. Return null
+     * otherwise.
+     */
+    synchronized String maybeName(NestedSetView<Artifact> view) {
+      if (reportedArtifactNames.containsKey(view.identifier())) {
+        return null;
+      }
+      Long name = nextArtifactName;
+      nextArtifactName++;
+      reportedArtifactNames.put(view.identifier(), name);
+      return name.toString();
+    }
+  }
+
   public BuildEventStreamer(Collection<BuildEventTransport> transports) {
     this.transports = transports;
     this.announcedEvents = null;
@@ -93,9 +133,9 @@ public class BuildEventStreamer implements EventHandler {
 
     for (BuildEventTransport transport : transports) {
       if (linkEvent != null) {
-        transport.sendBuildEvent(linkEvent);
+        transport.sendBuildEvent(linkEvent, artifactGroupNamer);
       }
-      transport.sendBuildEvent(event);
+      transport.sendBuildEvent(event, artifactGroupNamer);
     }
   }
 
@@ -142,6 +182,21 @@ public class BuildEventStreamer implements EventHandler {
     }
   }
 
+  private void maybeReportArtifactSet(NestedSetView<Artifact> view) {
+    String name = artifactGroupNamer.maybeName(view);
+    if (name == null) {
+      return;
+    }
+    for (NestedSetView<Artifact> transitive : view.transitives()) {
+      maybeReportArtifactSet(transitive);
+    }
+    post(new NamedArtifactGroup(name, view));
+  }
+
+  private void maybeReportArtifactSet(NestedSet<Artifact> set) {
+    maybeReportArtifactSet(new NestedSetView<Artifact>(set));
+  }
+
   @Override
   public void handle(Event event) {}
 
@@ -161,6 +216,13 @@ public class BuildEventStreamer implements EventHandler {
       return;
     }
 
+    if (event instanceof EventReportingArtifacts) {
+      for (NestedSet<Artifact> artifactSet :
+          ((EventReportingArtifacts) event).reportedArtifacts()) {
+        maybeReportArtifactSet(artifactSet);
+      }
+    }
+
     post(event);
 
     // Reconsider all events blocked by the event just posted.
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/NamedArtifactGroup.java b/src/main/java/com/google/devtools/build/lib/runtime/NamedArtifactGroup.java
new file mode 100644
index 0000000000..601f39d4dd
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/runtime/NamedArtifactGroup.java
@@ -0,0 +1,71 @@
+// Copyright 2016 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.runtime;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.buildeventstream.ArtifactGroupNamer;
+import com.google.devtools.build.lib.buildeventstream.BuildEvent;
+import com.google.devtools.build.lib.buildeventstream.BuildEventConverters;
+import com.google.devtools.build.lib.buildeventstream.BuildEventId;
+import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos;
+import com.google.devtools.build.lib.buildeventstream.GenericBuildEvent;
+import com.google.devtools.build.lib.buildeventstream.PathConverter;
+import com.google.devtools.build.lib.collect.nestedset.NestedSetView;
+import java.util.Collection;
+
+/**
+ * A {@link BuildEvent} introducing a set of artifacts to be referred to later by its name. Those
+ * events are generated by the {@link BuildEventStreamer} upon seeing an {@link
+ * EventReportingAritfacts}, if necessary.
+ */
+class NamedArtifactGroup implements BuildEvent {
+  private final String name;
+  private final NestedSetView<Artifact> view;
+
+  NamedArtifactGroup(String name, NestedSetView<Artifact> view) {
+    this.name = name;
+    this.view = view;
+  }
+
+  @Override
+  public BuildEventId getEventId() {
+    return BuildEventId.fromArtifactGroupName(name);
+  }
+
+  @Override
+  public Collection<BuildEventId> getChildrenEvents() {
+    return ImmutableSet.<BuildEventId>of();
+  }
+
+  @Override
+  public BuildEventStreamProtos.BuildEvent asStreamProto(BuildEventConverters converters) {
+    PathConverter pathConverter = converters.pathConverter();
+    ArtifactGroupNamer namer = converters.artifactGroupNamer();
+
+    BuildEventStreamProtos.NamedSetOfFiles.Builder builder =
+        BuildEventStreamProtos.NamedSetOfFiles.newBuilder();
+    for (Artifact artifact : view.directs()) {
+      BuildEventStreamProtos.File.Builder fileBuilder = BuildEventStreamProtos.File.newBuilder();
+      String name = artifact.getFilename();
+      String uri = pathConverter.apply(artifact.getPath());
+      builder.addFiles(fileBuilder.setName(name).setUri(uri).build());
+    }
+    for (NestedSetView<Artifact> child : view.transitives()) {
+      builder.addFileSets(namer.apply(child.identifier()));
+    }
+    return GenericBuildEvent.protoChaining(this).setNamedSetOfFiles(builder.build()).build();
+  }
+}