change incremental dexing fallback in DexFileMerger to behave exactly as if classes

were converted to dex.

--
PiperOrigin-RevId: 148255629
MOS_MIGRATED_REVID=148255629

3 files changed, 7 insertions, 140 deletions

diff --git a/src/tools/android/java/com/google/devtools/build/android/dexer/DexFileAggregator.java b/src/tools/android/java/com/google/devtools/build/android/dexer/DexFileAggregator.java
index d481323b06..53e2bfcbb1 100644
--- a/src/tools/android/java/com/google/devtools/build/android/dexer/DexFileAggregator.java
+++ b/src/tools/android/java/com/google/devtools/build/android/dexer/DexFileAggregator.java
@@ -115,12 +115,9 @@ class DexFileAggregator implements Closeable {
       addSeparate(dexFile);
     } else {
       // Could be smarter here and e.g. check whether dexFile will fit into current shard first
-      // and, with a hint as to whether this is a "full" file (e.g., from rotating in MergingDexer),
-      // write the current shard and then the given file separately.  The following is slower but
-      // guarantees that in the MINIMAL case, classes are written in the order in which we saw them
-      // as best as possible.  Since practically we aim for this method to never or almost never
-      // be called it shouldn't matter much.  (This method is called when we had to convert .class
-      // files on the fly, while add(Dex) is in particular called for pre-dexed .class.dex files.)
+      // and, with a hint as to whether this is a "full" file, write the current shard and then the
+      // given file separately.  The following is slower but guarantees that in the MINIMAL case,
+      // classes are written in the order in which we saw them as best as possible.
       add(DexFiles.toDex(dexFile));
     }
     return this;
diff --git a/src/tools/android/java/com/google/devtools/build/android/dexer/DexFileMerger.java b/src/tools/android/java/com/google/devtools/build/android/dexer/DexFileMerger.java
index ae535a4b4c..5f6f265af2 100644
--- a/src/tools/android/java/com/google/devtools/build/android/dexer/DexFileMerger.java
+++ b/src/tools/android/java/com/google/devtools/build/android/dexer/DexFileMerger.java
@@ -151,12 +151,7 @@ class DexFileMerger {
         System.setOut(DxConsole.noop);
       }
 
-      MergingDexer dexer =
-          new MergingDexer(
-              new Dexing(dexingOptions),
-              out,
-              options.multidexMode.isMultidexAllowed(),
-              options.maxNumberOfIdxPerDex);
+      DexConverter dexer = new DexConverter(new Dexing(dexingOptions));
       if (classesInMainDex == null) {
         processClassAndDexFiles(zip, out, dexer, Predicates.<ZipEntry>alwaysTrue());
       } else {
@@ -169,7 +164,6 @@ class DexFileMerger {
         // 2. process the remaining files
         Predicate<ZipEntry> classFileFilter = ZipEntryPredicates.classFileFilter(classesInMainDex);
         processClassAndDexFiles(zip, out, dexer, classFileFilter);
-        dexer.flush(); // Add any main dex list classes we had to convert on-the-fly
         // Fail if main_dex_list is too big, following dx's example
         checkState(out.getDexFilesWritten() == 0, "Too many classes listed in main dex list file "
             + "%s, main dex capacity exceeded", options.mainDexListFile);
@@ -178,8 +172,6 @@ class DexFileMerger {
         }
         processClassAndDexFiles(zip, out, dexer, Predicates.not(classFileFilter));
       }
-      // Add any classes to output archive that we had to convert on-the-fly
-      dexer.finish();
     } finally {
       System.setOut(originalStdOut);
     }
@@ -191,7 +183,7 @@ class DexFileMerger {
   private static void processClassAndDexFiles(
       ZipFile zip,
       DexFileAggregator out,
-      MergingDexer dexer,
+      DexConverter dexer,
       Predicate<ZipEntry> extraFilter)
       throws IOException {
     @SuppressWarnings("unchecked") // Predicates.and uses varargs parameter with generics
@@ -213,7 +205,8 @@ class DexFileMerger {
           // a byte buffer before effectively calling Dex(byte[]) anyway.
           out.add(new Dex(ByteStreams.toByteArray(content)));
         } else if (filename.endsWith(".class")) {
-          dexer.add(Dexing.parseClassFile(ByteStreams.toByteArray(content), filename));
+          // TODO(b/34949364): Remove this fallback once Blaze incrementally dexes all Jars
+          out.add(DexFiles.toDex(dexer.toDexFile(ByteStreams.toByteArray(content), filename)));
         } else {
           throw new IllegalStateException("Shouldn't get here: " + filename);
         }
diff --git a/src/tools/android/java/com/google/devtools/build/android/dexer/MergingDexer.java b/src/tools/android/java/com/google/devtools/build/android/dexer/MergingDexer.java
deleted file mode 100644
index a102b9ccc5..0000000000
--- a/src/tools/android/java/com/google/devtools/build/android/dexer/MergingDexer.java
+++ /dev/null
@@ -1,123 +0,0 @@
-// 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.android.dexer;
-
-import static com.google.common.base.Preconditions.checkState;
-
-import com.android.dx.cf.direct.DirectClassFile;
-import com.android.dx.dex.file.DexFile;
-import java.io.IOException;
-
-class MergingDexer {
-
-  // NB: The following two constants are copied from com.android.dx.command.dexer.Main
-
-  /**
-   * Maximum number of methods added during dexing:
-   * <ul>
-   * <li>Array.newInstance may be added by RopperMachine,
-   * <li>ArrayIndexOutOfBoundsException.<init> may be added by EscapeAnalysis
-   * </ul>
-   */
-  private static final int MAX_METHOD_ADDED_DURING_DEX_CREATION = 2;
-
-  /** Maximum number of fields added during dexing: &lt;primitive types box class&gt;.TYPE. */
-  private static final int MAX_FIELD_ADDED_DURING_DEX_CREATION = 9;
-
-  private final int maxNumberOfIdxPerDex;
-  private final Dexing dexing;
-  private final DexFileAggregator dest;
-  private final boolean multidex;
-  private DexFile currentShard;
-
-  public MergingDexer(
-      Dexing dexing,
-      DexFileAggregator dest,
-      boolean multidex,
-      int maxNumberOfIdxPerDex) {
-    this.dexing = dexing;
-    this.dest = dest;
-    this.multidex = multidex;
-    this.maxNumberOfIdxPerDex = maxNumberOfIdxPerDex;
-    currentShard = dexing.newDexFile();
-  }
-
-  public MergingDexer add(DirectClassFile classFile) throws IOException {
-    if (multidex && !currentShard.isEmpty()) {
-      // NB: This code is copied from com.android.dx.command.dexer.Main
-
-      // Calculate max number of indices this class will add to the
-      // dex file.
-      // The possibility of overloading means that we can't easily
-      // know how many constant are needed for declared methods and
-      // fields. We therefore make the simplifying assumption that
-      // all constants are external method or field references.
-
-      int constantPoolSize = classFile.getConstantPool().size();
-      int maxMethodIdsInClass = constantPoolSize + classFile.getMethods().size()
-              + MAX_METHOD_ADDED_DURING_DEX_CREATION;
-      int maxFieldIdsInClass = constantPoolSize + classFile.getFields().size()
-              + MAX_FIELD_ADDED_DURING_DEX_CREATION;
-
-      if (maxNumberOfIdxPerDex < getCurrentShardFieldCount() + maxFieldIdsInClass
-          || maxNumberOfIdxPerDex < getCurrentShardMethodCount() + maxMethodIdsInClass) {
-        // For simplicity just start a new shard to fit the given file
-        // Don't bother with waiting for a later file that might fit the old shard as in the extreme
-        // we'd have to wait until the end to write all shards.
-        rotateDexFile();
-      }
-    }
-
-    dexing.addToDexFile(currentShard, classFile);
-    return this;
-  }
-
-  public void finish() throws IOException {
-    if (currentShard != null && !currentShard.isEmpty()) {
-      dest.add(currentShard);
-    }
-    currentShard = null;
-  }
-
-  public void flush() throws IOException {
-    checkState(multidex);
-    if (!currentShard.isEmpty()) {
-      dest.add(currentShard);
-    }
-    currentShard = dexing.newDexFile();
-  }
-
-  private void rotateDexFile() throws IOException {
-    checkState(multidex);
-    checkState(!currentShard.isEmpty());
-    // We could take advantage here of knowing that currentShard is "full" and force dest to just
-    // write it out instead of it trying to merge currentShard with other .dex files.
-    // We're not doing that for the moment because that can cause problems when processing a
-    // main_dex_list: if dest's currentShard still contains classes from main_dex_list then writing
-    // our currentShard (with classes not from main dex list) separately would cause dest to write
-    // classes.dex with our currentShard and classes from main_dex_list to end up in classes2.dex or
-    // later, unless we prevent that case somehow (e.g., by knowing that order matters when writing
-    // the first shard).
-    dest.add(currentShard);
-    currentShard = dexing.newDexFile();
-  }
-
-  private int getCurrentShardMethodCount() {
-    return currentShard.getMethodIds().items().size();
-  }
-
-  private int getCurrentShardFieldCount() {
-    return currentShard.getFieldIds().items().size();
-  }
-}