Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8263561: Re-examine uses of LinkedList #2744

Closed
wants to merge 5 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -40,7 +40,6 @@
import java.util.Arrays;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.ListIterator;
import java.util.Map;
@@ -242,7 +241,7 @@ private static void checkReturnTypes(List<ProxyMethod> methods) {
* List of return types that are not yet known to be
* assignable from ("covered" by) any of the others.
*/
LinkedList<Class<?>> uncoveredReturnTypes = new LinkedList<>();
List<Class<?>> uncoveredReturnTypes = new ArrayList<>(1);

nextNewReturnType:
for (ProxyMethod pm : methods) {
@@ -2887,7 +2887,7 @@ protected List<Locale> createObject(BaseLocale base) {
if (language.equals("nb") || isNorwegianBokmal) {
List<Locale> tmpList = getDefaultList("nb", script, region, variant);
// Insert a locale replacing "nb" with "no" for every list entry with precedence
List<Locale> bokmalList = new LinkedList<>();
List<Locale> bokmalList = new ArrayList<>();
for (Locale l_nb : tmpList) {
var isRoot = l_nb.getLanguage().isEmpty();
var l_no = Locale.getInstance(isRoot ? "" : "no",
@@ -2934,15 +2934,15 @@ private static List<Locale> getDefaultList(String language, String script, Strin
List<String> variants = null;

if (!variant.isEmpty()) {
variants = new LinkedList<>();
variants = new ArrayList<>();
int idx = variant.length();
while (idx != -1) {
variants.add(variant.substring(0, idx));
idx = variant.lastIndexOf('_', --idx);
}
}

List<Locale> list = new LinkedList<>();
List<Locale> list = new ArrayList<>();

if (variants != null) {
for (String v : variants) {
@@ -54,7 +54,6 @@
import java.util.Enumeration;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.NoSuchElementException;
import java.util.Properties;
@@ -218,7 +217,7 @@ public synchronized List<IOException> closeLoaders() {
if (closed) {
return Collections.emptyList();
}
List<IOException> result = new LinkedList<>();
List<IOException> result = new ArrayList<>();
stsypanov marked this conversation as resolved.
Show resolved Hide resolved
for (Loader loader : loaders) {
try {
loader.close();
@@ -954,7 +953,7 @@ Resource getResource(final String name, boolean check,
Resource res;
String[] jarFiles;
int count = 0;
LinkedList<String> jarFilesList = null;
List<String> jarFilesList;

/* If there no jar files in the index that can potential contain
* this resource then return immediately.
@@ -52,13 +52,13 @@ public class JarIndex {
* The hash map that maintains mappings from
* package/classe/resource to jar file list(s)
*/
private HashMap<String,LinkedList<String>> indexMap;
private final HashMap<String, List<String>> indexMap;

/**
* The hash map that maintains mappings from
* jar file to package/class/resource lists
*/
private HashMap<String,LinkedList<String>> jarMap;
private final HashMap<String, List<String>> jarMap;

/*
* An ordered list of jar file names.
@@ -132,13 +132,13 @@ public String[] getJarFiles() {

/*
* Add the key, value pair to the hashmap, the value will
* be put in a linked list which is created if necessary.
* be put in a list which is created if necessary.
*/
private void addToList(String key, String value,
HashMap<String,LinkedList<String>> t) {
LinkedList<String> list = t.get(key);
HashMap<String, List<String>> t) {
List<String> list = t.get(key);
if (list == null) {
list = new LinkedList<>();
list = new ArrayList<>(1);
list.add(value);
t.put(key, list);
} else if (!list.contains(value)) {
@@ -151,8 +151,8 @@ private void addToList(String key, String value,
*
* @param fileName the key of the mapping
*/
public LinkedList<String> get(String fileName) {
LinkedList<String> jarFiles = null;
public List<String> get(String fileName) {
Copy link

@Thihup Thihup May 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@AlanBateman AlanBateman May 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IcedTea-Web seems to be using this method reflectively:
https://github.com/AdoptOpenJDK/IcedTea-Web/blob/master/common/src/main/java/net/adoptopenjdk/icedteaweb/jdk89access/JarIndexAccess.java#L80

I assume this doesn't work with JDK 16, at least not without using --add-exports to export jdk.internal.util.jar.

Copy link

@Thihup Thihup May 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

@stsypanov stsypanov May 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we then revert the changes to JarIndex?

Copy link
Contributor

@liach liach May 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But don't people access these internal code at their own risk, as jdk may change these code at any time without notice?

Copy link
Contributor Author

@stsypanov stsypanov May 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, I just wonder whether it's OK to change internals when we know for sure that this breaks 3rd party code

Copy link
Contributor

@liach liach May 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so. There are always unexpected ways the jdk may break and third-party libraries would need a different workaround for a new java version. For instance, in Apache log4j, its library has a special guard against the broken implementation of Reflection getCallerClass during java 7. The libraries can just handle these in their version-specific components.

Especially given the fact that the code linked above already has version-specific handling (8 vs 9), so it won't hurt much for them to add another piece of logic to handle jdk 17+ in case this optimization is merged.

List<String> jarFiles;
if ((jarFiles = indexMap.get(fileName)) == null) {
/* try the package name again */
int pos;
@@ -166,7 +166,7 @@ public LinkedList<String> get(String fileName) {
/**
* Add the mapping from the specified file to the specified
* jar file. If there were no mapping for the package of the
* specified file before, a new linked list will be created,
* specified file before, a new list will be created,
* the jar file is added to the list and a new mapping from
* the package to the jar file list is added to the hashmap.
* Otherwise, the jar file will be added to the end of the
@@ -261,7 +261,7 @@ public void write(OutputStream out) throws IOException {
/* print out the jar file name */
String jar = jarFiles[i];
bw.write(jar + "\n");
LinkedList<String> jarlist = jarMap.get(jar);
List<String> jarlist = jarMap.get(jar);
if (jarlist != null) {
Iterator<String> listitr = jarlist.iterator();
while(listitr.hasNext()) {
@@ -288,7 +288,7 @@ public void read(InputStream is) throws IOException {
String currentJar = null;

/* an ordered list of jar file names */
Vector<String> jars = new Vector<>();
List<String> jars = new ArrayList<>();

/* read until we see a .jar line */
while((line = br.readLine()) != null && !line.endsWith(".jar"));
@@ -320,11 +320,11 @@ public void read(InputStream is) throws IOException {
*
*/
public void merge(JarIndex toIndex, String path) {
Iterator<Map.Entry<String,LinkedList<String>>> itr = indexMap.entrySet().iterator();
Iterator<Map.Entry<String, List<String>>> itr = indexMap.entrySet().iterator();
while(itr.hasNext()) {
Map.Entry<String,LinkedList<String>> e = itr.next();
Map.Entry<String, List<String>> e = itr.next();
String packageName = e.getKey();
LinkedList<String> from_list = e.getValue();
List<String> from_list = e.getValue();
Iterator<String> listItr = from_list.iterator();
while(listItr.hasNext()) {
String jarName = listItr.next();
@@ -30,7 +30,7 @@
import java.nio.channels.MembershipKey;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;

@@ -94,7 +94,7 @@ void add(MembershipKeyImpl key) {
keys = groups.get(group);
}
if (keys == null) {
keys = new LinkedList<>();
keys = new ArrayList<>();
groups.put(group, keys);
}
keys.add(key);
@@ -40,14 +40,14 @@

abstract class AbstractPoller implements Runnable {

// list of requests pending to the poller thread
private final LinkedList<Request> requestList;
// requests pending to the poller thread
private final ArrayDeque<Request> requests;

// set to true when shutdown
private boolean shutdown;

protected AbstractPoller() {
this.requestList = new LinkedList<>();
this.requests = new ArrayDeque<>();
this.shutdown = false;
}

@@ -215,11 +215,11 @@ Object awaitResult() {
private Object invoke(RequestType type, Object... params) throws IOException {
// submit request
Request req = new Request(type, params);
synchronized (requestList) {
synchronized (requests) {
if (shutdown) {
throw new ClosedWatchServiceException();
}
requestList.add(req);
requests.add(req);

// wakeup thread
wakeup();
@@ -242,9 +242,9 @@ private Object invoke(RequestType type, Object... params) throws IOException {
*/
@SuppressWarnings("unchecked")
boolean processRequests() {
synchronized (requestList) {
synchronized (requests) {
Request req;
while ((req = requestList.poll()) != null) {
while ((req = requests.poll()) != null) {
// if in process of shutdown then reject request
if (shutdown) {
req.release(new ClosedWatchServiceException());
@@ -26,7 +26,7 @@
package sun.util.locale.provider;

import java.lang.ref.SoftReference;
import java.util.LinkedList;
import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import java.util.Map;
@@ -98,7 +98,7 @@ private static String[][] loadZoneStrings(Locale locale) {

// Performs per-ID retrieval.
Set<String> zoneIDs = LocaleProviderAdapter.forJRE().getLocaleResources(locale).getZoneIDs();
List<String[]> zones = new LinkedList<>();
List<String[]> zones = new ArrayList<>();
for (String key : zoneIDs) {
String[] names = retrieveDisplayNamesImpl(key, locale);
if (names != null) {
@@ -26,7 +26,7 @@
package sun.net.dns;

import java.util.List;
import java.util.LinkedList;
import java.util.ArrayList;
import java.util.StringTokenizer;
import java.io.BufferedReader;
import java.io.FileReader;
@@ -56,11 +56,11 @@
// Parse /etc/resolv.conf to get the values for a particular
// keyword.
//
private LinkedList<String> resolvconf(String keyword,
private ArrayList<String> resolvconf(String keyword,
int maxperkeyword,
int maxkeywords)
{
LinkedList<String> ll = new LinkedList<>();
ArrayList<String> ll = new ArrayList<>();

try {
BufferedReader in =
@@ -111,8 +111,8 @@ private LinkedList<String> resolvconf(String keyword,
return ll;
}

private LinkedList<String> searchlist;
private LinkedList<String> nameservers;
private ArrayList<String> searchlist;
private ArrayList<String> nameservers;


// Load DNS configuration from OS
@@ -132,7 +132,7 @@ private void loadConfig() {
nameservers =
java.security.AccessController.doPrivileged(
new java.security.PrivilegedAction<>() {
public LinkedList<String> run() {
public ArrayList<String> run() {
// typically MAXNS is 3 but we've picked 5 here
// to allow for additional servers if required.
return resolvconf("nameserver", 1, 5);
@@ -149,16 +149,16 @@ public LinkedList<String> run() {

// obtain search list or local domain

private LinkedList<String> getSearchList() {
private ArrayList<String> getSearchList() {

LinkedList<String> sl;
ArrayList<String> sl;

// first try the search keyword in /etc/resolv.conf

sl = java.security.AccessController.doPrivileged(
new java.security.PrivilegedAction<>() {
public LinkedList<String> run() {
LinkedList<String> ll;
public ArrayList<String> run() {
ArrayList<String> ll;

// first try search keyword (max 6 domains)
ll = resolvconf("search", 6, 1);
@@ -181,8 +181,8 @@ public LinkedList<String> run() {

sl = java.security.AccessController.doPrivileged(
new java.security.PrivilegedAction<>() {
public LinkedList<String> run() {
LinkedList<String> ll;
public ArrayList<String> run() {
ArrayList<String> ll;

ll = resolvconf("domain", 1, 1);
if (ll.size() > 0) {
@@ -199,7 +199,7 @@ public LinkedList<String> run() {
// no local domain so try fallback (RPC) domain or
// hostName

sl = new LinkedList<>();
sl = new ArrayList<>();
String domain = fallbackDomain0();
if (domain != null && !domain.isEmpty()) {
sl.add(domain);
@@ -35,7 +35,7 @@
import java.io.FileNotFoundException;
import java.io.FileOutputStream;
import java.io.IOException;
import java.util.LinkedList;
import java.util.List;
import java.util.jar.JarEntry;
import java.util.jar.JarOutputStream;
// implementation specific API
@@ -64,7 +64,7 @@ public static void main(String[] args) throws Exception {
static void assertFileResolved(JarIndex jarIndex2, String file,
String jarName) {
@SuppressWarnings("unchecked")
LinkedList<String> jarLists = (LinkedList<String>)jarIndex2.get(file);
List<String> jarLists = (List<String>)jarIndex2.get(file);
if (jarLists == null || jarLists.size() == 0 ||
!jarName.equals(jarLists.get(0))) {
throw new RuntimeException(