Skip to content

Commit

Permalink
Prefer ArrayList over LinkedList
Browse files Browse the repository at this point in the history 
ArrayList uses has less space overhead and better lookup time.
LinkedList is only better when we need to add or remove items
from somewhere besides the end of the list. This never happens
in our code base in practice. Uses of LinkedList should be viewed
with suspicion since it's easy to performance bottlenecks through
use of #get(int index) on a long linked list, as was fixed in
#6004

This PR changes all but two usages of LinkedList to ArrayList.
All usages for the lists were inspected to make sure we weren't
using any features of the list that would make LinkedList more
appropriate than ArrayList.
  • Loading branch information
@aaudiber
aaudiber committed Sep 2, 2017
1 parent de528ee commit 1e45a10
Show file tree
Hide file tree
Showing 17 changed files with 52 additions and 51 deletions.
4 changes: 2 additions & 2 deletions core/client/fs/src/main/java/alluxio/client/file/FileOutStream.java
View file
Expand Up @@ -36,7 +36,7 @@
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.util.LinkedList;
import java.util.ArrayList;
import java.util.List;

import javax.annotation.concurrent.NotThreadSafe;
Expand Down Expand Up @@ -90,7 +90,7 @@ public FileOutStream(AlluxioURI path, OutStreamOptions options, FileSystemContex
mOptions = options;
mContext = context;
mBlockStore = AlluxioBlockStore.create(mContext);
mPreviousBlockOutStreams = new LinkedList<>();
mPreviousBlockOutStreams = new ArrayList<>();
mClosed = false;
mCanceled = false;
mShouldCacheCurrentBlock = mAlluxioStorageType.isStore();
Expand Down
2 changes: 1 addition & 1 deletion core/common/src/main/java/alluxio/Configuration.java
View file
Expand Up @@ -320,7 +320,7 @@ public static List<String> getList(PropertyKey key, String delimiter) {
"Illegal separator for Alluxio properties as list");
String rawValue = get(key);

return Lists.newLinkedList(Splitter.on(delimiter).trimResults().omitEmptyStrings()
return Lists.newArrayList(Splitter.on(delimiter).trimResults().omitEmptyStrings()
.split(rawValue));
}

Expand Down
9 changes: 4 additions & 5 deletions core/common/src/main/java/alluxio/underfs/ObjectUnderFileSystem.java
View file
Expand Up @@ -36,7 +36,6 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.concurrent.Callable;
Expand Down Expand Up @@ -293,7 +292,7 @@ protected class DeleteBuffer {
public DeleteBuffer() {
mBatches = new ArrayList<>();
mBatchesResult = new ArrayList<>();
mCurrentBatchBuffer = new LinkedList<>();
mCurrentBatchBuffer = new ArrayList<>();
mEntriesAdded = 0;
}

Expand Down Expand Up @@ -321,7 +320,7 @@ public void add(String path) throws IOException {
*/
public List<String> getResult() throws IOException {
submitBatch();
LinkedList<String> result = new LinkedList<>();
List<String> result = new ArrayList<>();
for (Future<List<String>> list : mBatchesResult) {
try {
result.addAll(list.get());
Expand All @@ -345,7 +344,7 @@ public List<String> getResult() throws IOException {
private void submitBatch() throws IOException {
if (mCurrentBatchBuffer.size() != 0) {
int batchNumber = mBatches.size();
mBatches.add(new LinkedList<>(mCurrentBatchBuffer));
mBatches.add(new ArrayList<>(mCurrentBatchBuffer));
mCurrentBatchBuffer.clear();
mBatchesResult.add(batchNumber,
mExecutorService.submit(new DeleteThread(mBatches.get(batchNumber))));
Expand Down Expand Up @@ -619,7 +618,7 @@ protected String convertToFolderName(String key) {
* @return list of successfully deleted keys
*/
protected List<String> deleteObjects(List<String> keys) throws IOException {
List<String> result = new LinkedList<>();
List<String> result = new ArrayList<>();
for (String key : keys) {
boolean status = deleteObject(key);
// If key is a directory, it is possible that it was not created through Alluxio and no
Expand Down
5 changes: 3 additions & 2 deletions core/common/src/test/java/alluxio/heartbeat/HeartbeatThreadTest.java
View file
Expand Up @@ -17,8 +17,9 @@
import org.junit.Test;

import javax.annotation.Nullable;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.LinkedList;
import java.util.List;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutorService;
Expand Down Expand Up @@ -91,7 +92,7 @@ public void serialHeartbeatThread() throws Exception {
*/
@Test
public void concurrentHeartbeatThread() throws Exception {
List<FutureTask<Void>> tasks = new LinkedList<>();
List<FutureTask<Void>> tasks = new ArrayList<>();

// Start the threads.
for (int i = 0; i < NUMBER_OF_THREADS; i++) {
Expand Down
8 changes: 4 additions & 4 deletions core/common/src/test/java/alluxio/util/CommonUtilsTest.java
View file
Expand Up @@ -28,7 +28,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.LinkedList;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.Callable;
import java.util.concurrent.CyclicBarrier;
Expand Down Expand Up @@ -85,7 +85,7 @@ public TestCase(String expected, Object... objs) {
}
}

List<TestCase> testCases = new LinkedList<>();
List<TestCase> testCases = new ArrayList<>();
testCases.add(new TestCase(""));
testCases.add(new TestCase("foo", "foo"));
testCases.add(new TestCase("foo bar", "foo", "bar"));
Expand All @@ -110,7 +110,7 @@ public TestCase(String... strings) {
}
}

List<TestCase> testCases = new LinkedList<>();
List<TestCase> testCases = new ArrayList<>();
testCases.add(new TestCase());
testCases.add(new TestCase("foo"));
testCases.add(new TestCase("foo", "bar"));
Expand Down Expand Up @@ -145,7 +145,7 @@ public TestCase(String expected, Class<?> cls, Class<?>[] ctorClassArgs, Object.
}
}

List<TestCase> testCases = new LinkedList<>();
List<TestCase> testCases = new ArrayList<>();
testCases.add(new TestCase("hello", TestClassA.class, null));
testCases.add(new TestCase("1", TestClassB.class, new Class[] {int.class}, 1));

Expand Down
12 changes: 6 additions & 6 deletions core/common/src/test/java/alluxio/util/FormatUtilsTest.java
View file
Expand Up @@ -17,7 +17,7 @@
import org.junit.Test;

import java.nio.ByteBuffer;
import java.util.LinkedList;
import java.util.ArrayList;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
Expand All @@ -42,7 +42,7 @@ public TestCase(String expected, Object[] objs) {
}
}

List<TestCase> testCases = new LinkedList<>();
List<TestCase> testCases = new ArrayList<>();
testCases.add(new TestCase("()", null));
testCases.add(new TestCase("(null)", new Object[] {null}));
testCases.add(new TestCase("()", new Object[] {""}));
Expand Down Expand Up @@ -74,7 +74,7 @@ public TestCase(String expected, ByteBuffer input) {
}
}

List<TestCase> testCases = new LinkedList<>();
List<TestCase> testCases = new ArrayList<>();
testCases.add(new TestCase("", ByteBuffer.wrap(new byte[] {})));
testCases.add(new TestCase("", ByteBuffer.wrap(new byte[] {0})));
testCases.add(new TestCase("", ByteBuffer.wrap(new byte[] {0, 0})));
Expand Down Expand Up @@ -117,7 +117,7 @@ public TestCase(String expectedRE, String inputMessage) {
}
}

List<TestCase> testCases = new LinkedList<>();
List<TestCase> testCases = new ArrayList<>();
testCases.add(new TestCase("^Task A took (.*) ms.$", "Task A"));
testCases.add(new TestCase("^Task B took (.*) ms.$", "Task B"));

Expand Down Expand Up @@ -147,7 +147,7 @@ public TestCase(String expectedRE, String inputMessage) {
}
}

List<TestCase> testCases = new LinkedList<>();
List<TestCase> testCases = new ArrayList<>();
testCases.add(new TestCase("^Task A took (.*) ns.$", "Task A"));
testCases.add(new TestCase("^Task B took (.*) ns.$", "Task B"));

Expand Down Expand Up @@ -177,7 +177,7 @@ public TestCase(String expected, long input) {
}
}

List<TestCase> testCases = new LinkedList<>();
List<TestCase> testCases = new ArrayList<>();
testCases.add(new TestCase("4.00B", 1L << 2));
testCases.add(new TestCase("8.00B", 1L << 3));
testCases.add(new TestCase("4096.00B", 1L << 12));
Expand Down
12 changes: 6 additions & 6 deletions core/common/src/test/java/alluxio/util/io/BufferUtilsTest.java
View file
Expand Up @@ -16,7 +16,7 @@

import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.LinkedList;
import java.util.ArrayList;
import java.util.List;

/**
Expand Down Expand Up @@ -132,7 +132,7 @@ public TestCase(byte expected, int input) {
}
}

LinkedList<TestCase> testCases = new LinkedList<>();
ArrayList<TestCase> testCases = new ArrayList<>();
testCases.add(new TestCase((byte) 0x00, 0x00));
testCases.add(new TestCase((byte) 0x12, 0x12));
testCases.add(new TestCase((byte) 0x34, 0x1234));
Expand Down Expand Up @@ -163,7 +163,7 @@ public TestCase(byte[] expected, int length, int start) {
}
}

LinkedList<TestCase> testCases = new LinkedList<>();
ArrayList<TestCase> testCases = new ArrayList<>();
testCases.add(new TestCase(new byte[] {}, 0, 0));
testCases.add(new TestCase(new byte[] {}, 0, 3));
testCases.add(new TestCase(new byte[] {0}, 1, 0));
Expand Down Expand Up @@ -199,7 +199,7 @@ public TestCase(boolean expected, byte[] array, int length, int start) {
}
}

LinkedList<TestCase> testCases = new LinkedList<>();
ArrayList<TestCase> testCases = new ArrayList<>();
testCases.add(new TestCase(false, null, 0, 0));
testCases.add(new TestCase(true, new byte[] {}, 0, 0));
testCases.add(new TestCase(false, new byte[] {1}, 0, 0));
Expand Down Expand Up @@ -240,7 +240,7 @@ public TestCase(ByteBuffer expected, int length, int start) {
}
}

LinkedList<TestCase> testCases = new LinkedList<>();
ArrayList<TestCase> testCases = new ArrayList<>();
testCases.add(new TestCase(ByteBuffer.wrap(new byte[] {}), 0, 0));
testCases.add(new TestCase(ByteBuffer.wrap(new byte[] {}), 0, 3));
testCases.add(new TestCase(ByteBuffer.wrap(new byte[] {0}), 1, 0));
Expand Down Expand Up @@ -276,7 +276,7 @@ public TestCase(boolean expected, ByteBuffer buffer, int length, int start) {
}
}

LinkedList<TestCase> testCases = new LinkedList<>();
ArrayList<TestCase> testCases = new ArrayList<>();
testCases.add(new TestCase(false, null, 0, 0));
testCases.add(new TestCase(true, ByteBuffer.wrap(new byte[] {}), 0, 0));
testCases.add(new TestCase(false, ByteBuffer.wrap(new byte[] {1}), 0, 0));
Expand Down
4 changes: 2 additions & 2 deletions core/common/src/test/java/alluxio/util/io/PathUtilsTest.java
View file
Expand Up @@ -20,7 +20,7 @@
import org.junit.Test;
import org.junit.rules.ExpectedException;

import java.util.LinkedList;
import java.util.ArrayList;

/**
* Tests for the {@link PathUtils} class.
Expand Down Expand Up @@ -316,7 +316,7 @@ public void validatePath() throws InvalidPathException {
PathUtils.validatePath("/foo/../bar/");

// check invalid paths
LinkedList<String> invalidPaths = new LinkedList<>();
ArrayList<String> invalidPaths = new ArrayList<>();
invalidPaths.add(null);
invalidPaths.add("");
invalidPaths.add(" /");
Expand Down
4 changes: 2 additions & 2 deletions core/server/common/src/main/java/alluxio/cli/extensions/command/InstallCommand.java
View file
Expand Up @@ -25,7 +25,7 @@
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.util.LinkedList;
import java.util.ArrayList;
import java.util.List;

import javax.annotation.concurrent.ThreadSafe;
Expand Down Expand Up @@ -65,7 +65,7 @@ public String getDescription() {
public int run(CommandLine cl) {
String uri = cl.getArgs()[0];
String extensionsDir = Configuration.get(PropertyKey.EXTENSIONS_DIR);
List<String> failedHosts = new LinkedList<>();
List<String> failedHosts = new ArrayList<>();
for (String host : ExtensionsShellUtils.getServerHostnames()) {
try {
LOG.info("Attempting to install extension on host {}", host);
Expand Down
4 changes: 2 additions & 2 deletions core/server/common/src/main/java/alluxio/cli/extensions/command/UninstallCommand.java
View file
Expand Up @@ -26,7 +26,7 @@
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.util.LinkedList;
import java.util.ArrayList;
import java.util.List;

import javax.annotation.concurrent.ThreadSafe;
Expand Down Expand Up @@ -66,7 +66,7 @@ public String getDescription() {
public int run(CommandLine cl) {
String uri = cl.getArgs()[0];
String extensionsDir = Configuration.get(PropertyKey.EXTENSIONS_DIR);
List<String> failedHosts = new LinkedList<>();
List<String> failedHosts = new ArrayList<>();
for (String host : ExtensionsShellUtils.getServerHostnames()) {
try {
LOG.info("Attempting to uninstall extension on host {}", host);
Expand Down
8 changes: 4 additions & 4 deletions core/server/master/src/main/java/alluxio/master/file/DefaultFileSystemMaster.java
View file
Expand Up @@ -143,7 +143,7 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -1379,7 +1379,7 @@ private List<Inode<?>> deleteInternal(LockedInodePath inodePath, boolean replaye
throw new InvalidPathException(ExceptionMessage.DELETE_ROOT_DIRECTORY.getMessage());
}

List<Pair<AlluxioURI, Inode>> delInodes = new LinkedList<>();
List<Pair<AlluxioURI, Inode>> delInodes = new ArrayList<>();
List<Inode<?>> deletedInodes = new ArrayList<>();

Pair<AlluxioURI, Inode> inodePair = new Pair<AlluxioURI, Inode>(inodePath.getUri(), inode);
Expand All @@ -1398,11 +1398,11 @@ private List<Inode<?>> deleteInternal(LockedInodePath inodePath, boolean replaye
ufsDeleter = new SafeUfsDeleter(mMountTable, delInodes, deleteOptions);
}
// Inodes to delete from tree after attempting to delete from UFS
List<Inode> inodesToDelete = new LinkedList<>();
List<Inode> inodesToDelete = new ArrayList<>();
// Inodes that are not safe for recursive deletes
Set<Long> unsafeInodes = new HashSet<>();
// Alluxio URIs which could not be deleted
List<String> failedUris = new LinkedList<>();
List<String> failedUris = new ArrayList<>();

TempInodePathForDescendant tempInodePath = new TempInodePathForDescendant(inodePath);
// We go through each inode, removing it from its parent set and from mDelInodes. If it's a
Expand Down
6 changes: 3 additions & 3 deletions core/server/master/src/main/java/alluxio/master/file/UfsSyncChecker.java
View file
Expand Up @@ -28,7 +28,7 @@
import java.util.Arrays;
import java.util.Comparator;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;

Expand Down Expand Up @@ -145,7 +145,7 @@ private UfsStatus[] getChildrenInUFS(AlluxioURI alluxioUri)
AlluxioURI curUri = ufsUri;
while (curUri != null) {
if (mListedDirectories.containsKey(curUri.toString())) {
List<UfsStatus> childrenList = new LinkedList<>();
List<UfsStatus> childrenList = new ArrayList<>();
for (UfsStatus childStatus : mListedDirectories.get(curUri.toString())) {
String childPath = PathUtils.concatPath(curUri, childStatus.getName());
String prefix = PathUtils.normalizePath(ufsUri.toString(), AlluxioURI.SEPARATOR);
Expand Down Expand Up @@ -176,7 +176,7 @@ private UfsStatus[] getChildrenInUFS(AlluxioURI alluxioUri)
* @return trimmed list, null if input is null
*/
private UfsStatus[] trimIndirect(UfsStatus[] children) {
List<UfsStatus> childrenList = new LinkedList<>();
List<UfsStatus> childrenList = new ArrayList<>();
for (UfsStatus child : children) {
int index = child.getName().indexOf(AlluxioURI.SEPARATOR);
if (index < 0 || index == child.getName().length()) {
Expand Down
1 change: 1 addition & 0 deletions core/server/master/src/main/java/alluxio/master/file/meta/InodeTree.java
View file
Expand Up @@ -54,6 +54,7 @@
import java.util.ArrayList;
import java.util.HashSet;
import java.util.Iterator;
import java.util.ArrayList;
import java.util.LinkedList;
import java.util.List;
import java.util.NoSuchElementException;
Expand Down
6 changes: 3 additions & 3 deletions core/server/master/src/test/java/alluxio/master/file/FileSystemMasterTest.java
View file
Expand Up @@ -86,7 +86,7 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;
import java.util.concurrent.ExecutorService;
Expand Down Expand Up @@ -363,7 +363,7 @@ public void deleteUnsyncedPersistedDirectoryWithCheck() throws Exception {
mFileSystemMaster.delete(new AlluxioURI(MOUNT_URI).join(DIR_TOP_LEVEL),
DeleteOptions.defaults().setRecursive(true).setAlluxioOnly(false).setUnchecked(false));
// Check all that could be deleted.
List<AlluxioURI> except = new LinkedList<>();
List<AlluxioURI> except = new ArrayList<>();
except.add(new AlluxioURI(MOUNT_URI).join(DIR_TOP_LEVEL));
checkPersistedDirectoriesDeleted(1, ufsMount, except);
}
Expand Down Expand Up @@ -403,7 +403,7 @@ public void deleteUnsyncedPersistedMultilevelDirectoryWithCheck() throws Excepti
mFileSystemMaster.delete(new AlluxioURI(MOUNT_URI).join(DIR_TOP_LEVEL),
DeleteOptions.defaults().setRecursive(true).setAlluxioOnly(false).setUnchecked(false));
// Check all that could be deleted.
List<AlluxioURI> except = new LinkedList<>();
List<AlluxioURI> except = new ArrayList<>();
except.add(new AlluxioURI(MOUNT_URI).join(DIR_TOP_LEVEL));
except.add(new AlluxioURI(MOUNT_URI).join(DIR_TOP_LEVEL).join(DIR_PREFIX + 0));
checkPersistedDirectoriesDeleted(3, ufsMount, except);
Expand Down

0 comments on commit 1e45a10

Please sign in to comment.