|
Quick Lists
|
|
Bug ID:
|
6337998
|
|
Votes
|
0
|
|
Synopsis
|
(coll) AbstractList.equals() uses Iterator which has a large performance cost
|
|
Category
|
java:classes_util
|
|
Reported Against
|
|
|
Release Fixed
|
|
|
State
|
5-Cause Known,
request for enhancement
|
|
Priority:
|
4-Low
|
|
Related Bugs
|
|
|
Submit Date
|
18-OCT-2005
|
|
Description
|
A DESCRIPTION OF THE REQUEST :
AbstractList.equals() uses Iterator which has a large performance cost because it instantiates two iterators. A for loop should be used to avoid instantiating objects.
JUSTIFICATION :
Code that uses Lists as keys for hashmaps ends up having the equals method invoked often. Under heavy load, this creates iterators only for the equals method, which consumes memory. By rewriting the equals method I was able to reduce memory consumption by 40% in my simple test case.
EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
I would like for the AbstractList.equals() method to use a for loop for the element comparisons instead of iterators.
ACTUAL -
The AbstractList.equals() uses iterators to invoke equals on each element in the two lists.
---------- BEGIN SOURCE ----------
import java.util.Arrays;
import junit.framework.TestCase;
public class ArrayListEqualsTest extends TestCase {
private static final int NUM_EXECUTIONS = 10;
@Override
protected void setUp() throws Exception {
super.setUp();
}
@Override
protected void tearDown() throws Exception {
super.tearDown();
}
public void testArrayListEquals() {
String[] args = new String[1];
args[0] = "asdfasdfasdf";
Object o1 = Arrays.asList(args);
Object o2 = Arrays.asList(args);
for (int i = 0; i < NUM_EXECUTIONS; i++) {
if (!o1.equals(o2)) {
System.out.println("not equal");
}
try {
Thread.sleep(100);
} catch (InterruptedException e) {
}
}
}
}
---------- END SOURCE ----------
CUSTOMER SUBMITTED WORKAROUND :
I did not override equals, instead I created a method to compare two lists..
public boolean listEquals(List l1, List l2) {
if (l1 == l2) {
return true;
}
if (l1.size() != l2.size()) {
return false;
}
for (int i = 0; i < l1.size(); i++) {
if (l1.get(i).equals(l2.get(i))) {
return false;
}
}
return true;
}
Posted Date : 2005-10-18 00:24:22.0
The submitter adds:
After submitting, I discovered that I missed a negation:
if (l1.get(i).equals(l2.get(i))) {
return false;
}
should be :
if (!(l1.get(i).equals(l2.get(i)))) {
return false;
}
Posted Date : 2005-10-23 21:08:27.0
|
|
Work Around
|
N/A
|
|
Evaluation
|
Doug Lea writes:
---------------------------------------------------------------------
The request as stated isn't a good idea, but might be made into one.
Lists/AbstractLists in general may have get(index) methods that
are slower than iterators would be (for example, this is true
of LinkedList). However, AbstractList methods and related code could
check for instances of RandomAccess and split out special-case
code accordingly. Although there are four cases (get, get),
(get, iterator), (iterator, get), and (iterator, iterator), which
might add more bulk-based overhead than it saves. A reasonable
compromise might be to only have the existing RandomAccess concrete
classes (ArrayList, Arrays.AsList, Vector) so something like this.
But the submitter has a much better solution available for
the particular code listed. Use Arrays.equals on the arrays
rather than transforming via asList at all.
---------------------------------------------------------------------
Posted Date : 2005-10-24 15:16:23.0
Contribution forum : https://jdk-collaboration.dev.java.net/servlets/ProjectForumMessageView?forumID=1463&messageID=23409
Posted Date : 2008-03-06 00:52:44.0
|
|
Comments
|
Submitted On 19-OCT-2005
cunparis
After submitting, I discovered that I missed a negation:
if (l1.get(i).equals(l2.get(i))) {
return false;
}
should be :
if (!(l1.get(i).equals(l2.get(i)))) {
return false;
}
PLEASE NOTE: JDK6 is formerly known as Project Mustang
|
|
|
 |