Refactor reducer (#7)

* Refactored reducer so that it is less fragment shader-centric.

* Added an additional test for empty block removal.

* Small refactoring to live output variable reduction opportunities.

common-util/src/main/java/com/graphicsfuzz/common/util/FileHelper.java

@@ -16,35 +16,16 @@
 
 package com.graphicsfuzz.common.util;
 
-import java.io.BufferedReader;
 import java.io.File;
 import java.io.FileNotFoundException;
-import java.io.FileReader;
-import java.io.IOException;
 import org.apache.commons.io.FilenameUtils;
 
 public class FileHelper {
 
-  public static File fileWithAppendedString(File file, String extra) {
-    String ext = FilenameUtils.getExtension(file.toString());
-    String basename = FilenameUtils.removeExtension(file.toString());
-    return new File(basename + extra + "." + ext);
-  }
-
-  public static String firstLine(File file) throws IOException {
-    try (BufferedReader r = new BufferedReader(new FileReader(file));) {
-      return r.readLine();
-    }
-  }
-
   public static File replaceExtension(File file, String ext) {
     return new File(FilenameUtils.removeExtension(file.toString()) + ext);
   }
 
-  public static File replaceDir(File file, File dir) {
-    return new File(dir, file.getName());
-  }
-
   public static void checkExists(File file) throws FileNotFoundException {
     if (!file.exists()) {
       throw new FileNotFoundException("Could not find: " + file);

common/src/main/java/com/graphicsfuzz/common/util/ReductionStepHelper.java → common/src/main/java/com/graphicsfuzz/common/util/ReductionProgressHelper.java

@@ -22,9 +22,9 @@
 import org.apache.commons.io.FilenameUtils;
 import org.apache.commons.lang3.StringUtils;
 
-public final class ReductionStepHelper {
+public final class ReductionProgressHelper {
 
-  private ReductionStepHelper() {
+  private ReductionProgressHelper() {
     // Utility class
   }
 
@@ -39,17 +39,18 @@ static int compareReductionTemporaryFiles(File first, File second, String shader
   }
 
   static boolean isAReductionStepFile(String name, String shaderPrefix, boolean restrictToSuccess) {
-    if (!name.startsWith(shaderPrefix)) {
+    final String shaderPrefixBasename = FilenameUtils.getBaseName(shaderPrefix);
+    if (!name.startsWith(shaderPrefixBasename)) {
       return false;
     }
     if (!name.contains("reduced")) {
       return false;
     }
-    if (!name.endsWith("success.frag")) {
+    if (!name.endsWith("success.json")) {
       if (restrictToSuccess) {
         return false;
       }
-      if (!name.endsWith("fail.frag")) {
+      if (!name.endsWith("fail.json")) {
         return false;
       }
     }
@@ -69,23 +70,28 @@ private static String[] getNameComponents(String name) {
 
   public static Optional<Integer> getLatestReductionStep(boolean restrictToSuccess,
         File reductionDir,
-        String shaderPrefix) {
-    final File[] fragmentShaders = reductionDir
-          .listFiles((dir, name) -> isAReductionStepFile(name, shaderPrefix, restrictToSuccess));
-    return Arrays.stream(fragmentShaders)
+        String shaderJobPrefix) {
+    final File[] jsonFiles = reductionDir
+          .listFiles((dir, name) -> isAReductionStepFile(name, shaderJobPrefix,
+              restrictToSuccess));
+    return Arrays.stream(jsonFiles)
           .map(item -> item.getName())
-          .max((item1, item2) -> compareReductionTemporaryFiles(item1, item2, shaderPrefix))
-          .flatMap(item -> Optional.of(getReductionStepFromFile(item, shaderPrefix)));
+          .max((item1, item2) -> compareReductionTemporaryFiles(item1, item2, shaderJobPrefix))
+          .flatMap(item -> Optional.of(getReductionStepFromFile(item, shaderJobPrefix)));
   }
 
   public static Optional<Integer> getLatestReductionStepAny(File reductionDir,
-        String shaderPrefix) {
-    return getLatestReductionStep(false, reductionDir, shaderPrefix);
+        String shaderJobPrefix) {
+    return getLatestReductionStep(false, reductionDir, shaderJobPrefix);
   }
 
   public static Optional<Integer> getLatestReductionStepSuccess(File reductionDir,
         String shaderPrefix) {
     return getLatestReductionStep(true, reductionDir, shaderPrefix);
   }
 
+  public static File getReductionExceptionFile(String shaderJobPrefix, File reductionDir) {
+    return new File(reductionDir, shaderJobPrefix + ".exception");
+  }
+
 }

reducer/src/main/java/com/graphicsfuzz/reducer/IReductionState.java

@@ -23,8 +23,6 @@
 
 public interface IReductionState {
 
-  UniformsInfo computeRemainingUniforms(File uniformsJson) throws FileNotFoundException;
-
   boolean hasFragmentShader();
 
   boolean hasVertexShader();
@@ -33,4 +31,6 @@ public interface IReductionState {
 
   TranslationUnit getVertexShader();
 
+  UniformsInfo getUniformsInfo();
+
 }

reducer/src/main/java/com/graphicsfuzz/reducer/IReductionStateFileWriter.java

@@ -20,7 +20,7 @@
 
 public interface IReductionStateFileWriter {
 
-  void writeFileFromState(IReductionState state, String outputFilesPrefix)
+  void writeFilesFromState(IReductionState state, String outputFilesPrefix)
       throws FileNotFoundException;
 
 }

reducer/src/main/java/com/graphicsfuzz/reducer/ReductionDriver.java

@@ -101,9 +101,8 @@ public String doReduction(
         LOGGER.info("Result from initial state is interesting - proceeding with reduction.");
       }
 
-      final String variantName = FilenameUtils.getBaseName(initialFilePrefix);
-
-      final File initialFileJson = new File(initialFilePrefix + ".json");
+      final String variantName = Paths.get(workDir.getAbsolutePath(),
+          FilenameUtils.getBaseName(initialFilePrefix)).toString();
 
       boolean isInteresting = true;
       int stepCount = 0;
@@ -116,12 +115,9 @@ public String doReduction(
         }
         ++stepCount;
         final int currentReductionAttempt = numReductionAttempts + fileCountOffset;
-        String outputFilesPrefix = Paths.get(workDir.getAbsolutePath(),
-            getReductionStepFilenamePrefix(variantName, currentReductionAttempt))
-            .toString();
-        fileWriter.writeFileFromState(newState, outputFilesPrefix);
-        Helper.emitUniformsInfo(newState.computeRemainingUniforms(initialFileJson),
-              new PrintStream(new FileOutputStream(outputFilesPrefix + ".json")));
+        String outputFilesPrefix = getReductionStepFilenamePrefix(variantName,
+            currentReductionAttempt);
+        fileWriter.writeFilesFromState(newState, outputFilesPrefix);
         isInteresting = isInterestingWithCache(judge, outputFilesPrefix);
         renameReductionStepFiles(isInteresting, variantName, currentReductionAttempt, workDir);
 
@@ -135,17 +131,13 @@ public String doReduction(
 
       IReductionState finalState = getSimplifiedState();
 
-      String finalOutputFilePrefix = Paths.get(workDir.getAbsolutePath(),
-          variantName + "_reduced_final").toString();
-      fileWriter.writeFileFromState(finalState, finalOutputFilePrefix);
-      Helper.emitUniformsInfo(finalState.computeRemainingUniforms(initialFileJson),
-            new PrintStream(new FileOutputStream(
-                  finalOutputFilePrefix + ".json")));
+      String finalOutputFilePrefix = variantName + "_reduced_final";
+      fileWriter.writeFilesFromState(finalState, finalOutputFilePrefix);
 
       if (!judge.isInteresting(finalOutputFilePrefix)) {
         LOGGER.info(
               "Failed to simplify final reduction state! Reverting to the non-simplified state.");
-        fileWriter.writeFileFromState(finalState, finalOutputFilePrefix);
+        fileWriter.writeFilesFromState(finalState, finalOutputFilePrefix);
       }
 
       if (stoppedEarly) {
@@ -195,11 +187,14 @@ private void renameReductionStepFiles(boolean isInteresting, String variantName,
         File workDir) throws IOException {
     for (String fileName : workDir.list((dir, name) -> FilenameUtils.removeExtension(name)
           .equals(getReductionStepFilenamePrefix(variantName, currentReductionAttempt)))) {
+      final File srcFile = new File(workDir, fileName);
+      final File dstFile = new File(workDir, getReductionStepFilenamePrefix(variantName,
+          currentReductionAttempt,
+          Optional.of(isInteresting ? "success" : "fail"))
+          + "." + FilenameUtils.getExtension(fileName));
       FileUtils.moveFile(
-            new File(workDir, fileName),
-            new File(workDir, getReductionStepFilenamePrefix(variantName, currentReductionAttempt,
-                  Optional.of(isInteresting ? "success" : "fail"))
-                  + "." + FilenameUtils.getExtension(fileName)));
+          srcFile,
+          dstFile);
     }
   }
 
@@ -298,13 +293,14 @@ private void giveUp() {
 
   public final GlslReductionState finaliseReduction() {
     // Do final cleanup pass to get rid of macros
-    // TODO: need to make this work for vertex shaders too.
+    final Optional<TranslationUnit> simplifiedVertexShader =
+        state.hasVertexShader() ? Optional.of(Simplify.simplify(state.getVertexShader()))
+            : Optional.empty();
     final Optional<TranslationUnit> simplifiedFragmentShader =
-        Optional.of(Simplify.simplify(state.getFragmentShader()));
-    Optional<TranslationUnit> vertexShader = state.hasVertexShader()
-        ? Optional.of(state.getVertexShader())
-        : Optional.empty();
-    return new GlslReductionState(simplifiedFragmentShader, vertexShader);
+        state.hasFragmentShader() ? Optional.of(Simplify.simplify(state.getFragmentShader()))
+            : Optional.empty();
+    return new GlslReductionState(simplifiedVertexShader, simplifiedFragmentShader,
+        state.getUniformsInfo());
   }
 
 

reducer/src/main/java/com/graphicsfuzz/reducer/filejudge/ImageShaderFileJudge.java

@@ -27,6 +27,7 @@
 import java.io.File;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Paths;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.FilenameUtils;
 import org.slf4j.Logger;
@@ -36,28 +37,28 @@ public class ImageShaderFileJudge implements IFileJudge {
 
   private static final Logger LOGGER = LoggerFactory.getLogger(ImageShaderFileJudge.class);
 
-  private File referenceShaderFile;
-  private File referenceShaderImage;
+  private static final String REFERENCE_IMAGE_NAME = "reference_image.png";
+
   private File workDir;
   private IImageFileComparator fileComparator;
   private IShaderDispatcher imageGenerator;
   private final boolean throwExceptionOnValidationError;
 
   public ImageShaderFileJudge(
-        File referenceShaderFile,
-        File referenceShaderImage,
         File workDir,
         IImageFileComparator fileComparator,
         IShaderDispatcher imageGenerator,
         boolean throwExceptionOnValidationError) {
-    this.referenceShaderFile = referenceShaderFile;
-    this.referenceShaderImage = referenceShaderImage;
     this.workDir = workDir;
     this.fileComparator = fileComparator;
     this.imageGenerator = imageGenerator;
     this.throwExceptionOnValidationError = throwExceptionOnValidationError;
   }
 
+  public static File getReferenceImageInWorkDir(File workDir) {
+    return Paths.get(workDir.toString(), REFERENCE_IMAGE_NAME).toFile();
+  }
+
   @Override
   public boolean isInteresting(String filesPrefix) throws FileJudgeException {
 
@@ -88,7 +89,7 @@ public boolean isInteresting(String filesPrefix) throws FileJudgeException {
                 outputText,
                 JobStatus.SAME_AS_REFERENCE.toString() + "\n",
                 StandardCharsets.UTF_8);
-          outputImage = referenceShaderImage;
+          outputImage = getReferenceImageInWorkDir(workDir);
           break;
         default:
           LOGGER.info("Failed to run get_image on shader. Not interesting.");
@@ -100,7 +101,7 @@ public boolean isInteresting(String filesPrefix) throws FileJudgeException {
       // Success or same as ref:
 
       // 3.
-      if (!fileComparator.areFilesInteresting(referenceShaderImage, outputImage)) {
+      if (!fileComparator.areFilesInteresting(getReferenceImageInWorkDir(workDir), outputImage)) {
         LOGGER.info("Shader image was not deemed interesting by file comparator. Not interesting.");
         return false;
       }

reducer/src/main/java/com/graphicsfuzz/reducer/glslreducers/GlslReductionState.java

@@ -31,17 +31,21 @@
 
 public class GlslReductionState implements IReductionState {
 
-  private final Optional<TranslationUnit> fragmentShader;
   private final Optional<TranslationUnit> vertexShader;
+  private final Optional<TranslationUnit> fragmentShader;
+  private final UniformsInfo uniformsInfo;
 
-  public GlslReductionState(Optional<TranslationUnit> fragmentShader,
-      Optional<TranslationUnit> vertexShader) {
-    this.fragmentShader = fragmentShader;
+  public GlslReductionState(Optional<TranslationUnit> vertexShader,
+                            Optional<TranslationUnit> fragmentShader,
+                            UniformsInfo uniformsInfo) {
     this.vertexShader = vertexShader;
+    this.fragmentShader = fragmentShader;
+    this.uniformsInfo = uniformsInfo;
   }
 
-  public GlslReductionState(Optional<TranslationUnit> fragmentShader) {
-    this(fragmentShader, Optional.empty());
+  @Override
+  public boolean hasVertexShader() {
+    return vertexShader.isPresent();
   }
 
   @Override
@@ -50,8 +54,9 @@ public boolean hasFragmentShader() {
   }
 
   @Override
-  public boolean hasVertexShader() {
-    return vertexShader.isPresent();
+  public TranslationUnit getVertexShader() {
+    assert hasVertexShader();
+    return vertexShader.get();
   }
 
   @Override
@@ -61,27 +66,8 @@ public TranslationUnit getFragmentShader() {
   }
 
   @Override
-  public TranslationUnit getVertexShader() {
-    assert hasVertexShader();
-    return vertexShader.get();
-  }
-
-  @Override
-  public UniformsInfo computeRemainingUniforms(File uniformsJson) throws FileNotFoundException {
-    assert fragmentShader.isPresent();
-    final UniformsInfo result = new UniformsInfo(
-          new File(uniformsJson.getAbsolutePath()));
-
-    Set<String> remainingUniforms = new HashSet<>();
-    remainingUniforms.addAll(getUniforms(fragmentShader));
-    remainingUniforms.addAll(getUniforms(vertexShader));
-
-    for (String name : result.getUniformNames().stream()
-        .filter(item -> !remainingUniforms.contains(item)).collect(
-        Collectors.toList())) {
-      result.removeUniform(name);
-    }
-    return result;
+  public UniformsInfo getUniformsInfo() {
+    return uniformsInfo;
   }
 
   private Set<String> getUniforms(Optional<TranslationUnit> maybeTu) {

reducer/src/main/java/com/graphicsfuzz/reducer/glslreducers/GlslReductionStateFileWriter.java

@@ -24,6 +24,7 @@
 import com.graphicsfuzz.reducer.IReductionState;
 import com.graphicsfuzz.reducer.IReductionStateFileWriter;
 import java.io.FileNotFoundException;
+import java.io.FileOutputStream;
 import java.io.PrintStream;
 
 public class GlslReductionStateFileWriter implements IReductionStateFileWriter {
@@ -35,14 +36,16 @@ public GlslReductionStateFileWriter(ShadingLanguageVersion shadingLanguageVersio
   }
 
   @Override
-  public void writeFileFromState(IReductionState state, String outputFilesPrefix)
+  public void writeFilesFromState(IReductionState state, String outputFilesPrefix)
         throws FileNotFoundException {
-    if (state.hasFragmentShader()) {
-      writeFile(state.getFragmentShader(), ShaderKind.FRAGMENT, outputFilesPrefix);
-    }
     if (state.hasVertexShader()) {
       writeFile(state.getVertexShader(), ShaderKind.VERTEX, outputFilesPrefix);
     }
+    if (state.hasFragmentShader()) {
+      writeFile(state.getFragmentShader(), ShaderKind.FRAGMENT, outputFilesPrefix);
+    }
+    Helper.emitUniformsInfo(state.getUniformsInfo(),
+        new PrintStream(new FileOutputStream(outputFilesPrefix + ".json")));
   }
 
   private void writeFile(TranslationUnit shader, ShaderKind shaderKind, String outputFilesPrefix)

reducer/src/main/java/com/graphicsfuzz/reducer/glslreducers/SimplePlan.java

@@ -251,7 +251,7 @@ private IReductionState stateFromWorkingShader(TranslationUnit workingShader,
       default:
         throw new RuntimeException("Unsupported shader kind: " + shaderKind);
     }
-    return new GlslReductionState(fragmentShader, vertexShader);
+    return new GlslReductionState(vertexShader, fragmentShader, originalState.getUniformsInfo());
   }
 
 }