Optimize trusted MutableList construction

Cuts back a lot of unnecessary copying. All construction is funneled through copyOf and
wrapUnsafe. copyOf is the traditional construction mechanism, taking defensive copies of
the input and determining if GlobList information needs to be retained. wrapUnsafe takes
full ownership of the supplied ArrayList, allowing us to skip a lot of copies in trusted
situations. This is particularly useful for common built in functions which return a list,
range() being one common example.

RELNOTES: None
PiperOrigin-RevId: 172361367

1 files changed, 27 insertions, 0 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkListTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkListTest.java
index 98695c56e0..b2768390da 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkListTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkListTest.java
@@ -17,9 +17,11 @@ import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.fail;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
 import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
 import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
 import com.google.devtools.build.lib.syntax.util.EvaluationTestCase;
+import java.util.ArrayList;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -275,4 +277,29 @@ public class SkylarkListTest extends EvaluationTestCase {
       assertThat(e).hasMessage("trying to mutate a frozen object");
     }
   }
+
+  @Test
+  public void testCopyOfTakesCopy() throws EvalException {
+    ArrayList<String> copyFrom = Lists.newArrayList("hi");
+    Mutability mutability = Mutability.create("test");
+    MutableList<String> mutableList = MutableList.copyOf(mutability, copyFrom);
+    copyFrom.add("added1");
+    mutableList.add("added2", /*loc=*/ null, mutability);
+
+    assertThat(copyFrom).containsExactly("hi", "added1").inOrder();
+    assertThat(mutableList).containsExactly("hi", "added2").inOrder();
+  }
+
+  @Test
+  public void testWrapUnsafeTakesOwnershipOfPassedArrayList() throws EvalException {
+    ArrayList<String> wrapped = Lists.newArrayList("hi");
+    Mutability mutability = Mutability.create("test");
+    MutableList<String> mutableList = MutableList.wrapUnsafe(mutability, wrapped);
+
+    // Big no-no, but we're proving a point.
+    wrapped.add("added1");
+    mutableList.add("added2", /*loc=*/ null, mutability);
+    assertThat(wrapped).containsExactly("hi", "added1", "added2").inOrder();
+    assertThat(mutableList).containsExactly("hi", "added1", "added2").inOrder();
+  }
 }