AW: [Patch] modifiedselector, style, remove unused code, slightly more lazy DigestAlgorithm.getValue (now with added source code -doh!)
- removed iterator code fromModifiedSelector Why you use the for-loop instead of iterator? - lazier DigestAlgorithm.getValue (includes JUnit test) - removed unncecessary nesting in EqualsComparator // // - Instantiate the interfaces - // String className = null; String pkg = org.apache.tools.ant.types.selectors.modifiedselector; what do these lines do in ModifiedSelector.configure? Eclipse says that they're never read, and as they're method variables, not class or instance variables (ie not public), I was very tempted to delete them, but I thought I'd better ask first in case they're present to support future functionality That came more from past functionality than for future one :) While developing the modified selector I started with reflection. And in earlier steps I had hardcoded classnames for the three interfaces as default. And so I could simply shorten the text to write. My ascii editor I used there couldn´t check for unneeded variables :) Jan
Re: AW: [Patch] modifiedselector, style, remove unused code, slightly more lazy DigestAlgorithm.getValue (now with added source code -doh!)
Why you use the for-loop instead of iterator?
1 iterator has to check underlying array bounds for every hasNext(), and
every next(), checking is slow, and in this case unecessary
2 iterator call to hasNext() each time around the loop - again we can
call size() once and cache the result, since we are never updating the
underlying structure, this won't change and the local var read is faster
than a method call.
3 iterator is a large object to create, an int is a small one
(bytecode is probably smaller, although I haven't checked this, I can
almost (99%) guarantee it)
4 remove calls to iterator, then we can remove the import :)
It's just an idiom I learnt from Effective Java or one of the books
like that, but the reasons above mean that I use it whenever I can, as
it does tend to speed up large loops, at the expense of only a very
slight obfuscation of the code. Some people use essentially the same
loop but move the size variable above the loop. That's ok, but it means
that the size variable has slightly larger scope than it requires,
although the effect is the same.
Also speaking of Iterators in general...
Iterators: Signs of Weakness in Object-Oriented Languages
http://citeseer.ist.psu.edu/55669.html (there's a PDF or PS file
downloadable from the link at the top)
I don't pretend to understand all the Lispness of this paper, but some
of the arguments against external iterators (the kind we have in Java)
are interesting. I did a little test last week comparing a normal
iterator/for loop, with a functional-style of programming, both in
Java. Speed was identical, but bytecode of the functional-style was
much smaller, there were less casts and less method calls. Anecdotal at
best, but try it yourself:
[I decided to call these things Closures, but they aren't quite, rather
more like Blocks, correct me if I'm getting my terms wrong]
public interface Closure {
Object execute(Object o);
}
import java.util.ArrayList;
import java.util.List;
public class ClosureUtils {
/**
* This method maps a function to a list of data
* and returns a new modified list.
* @param list the source list of data
* @param c the Closure to apply to the list
* @return the modified list
*/
public static List map(final List list, final Closure c) {
final int size = list.size();
List result = new ArrayList(size);
for (int i=0; isize; i++) {
result.add(c.execute(list.get(i)));
}
return result;
}
}
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Date;
import java.util.Iterator;
import java.util.List;
import junit.framework.TestCase;
public class ClosureUtilsTest extends TestCase {
List testList = Arrays.asList(new String[]{This, That, Other});
/*
* @see TestCase#setUp()
*/
protected void setUp() throws Exception {
super.setUp();
}
/*
* @see TestCase#tearDown()
*/
protected void tearDown() throws Exception {
super.tearDown();
}
public void testJ5() {
try {
Date start = new Date();
System.out.println(Start J5);
for (int i=0; i10; i++) {
List newList = new ArrayList();
for (Object o : testList) {
newList.add(((String)o).toUpperCase());
}
assertEquals(((String)newList.get(0)),
((String)testList.get(0)).toUpperCase());
}
System.out.println(End elapsed : [ +(new
Date().getTime()-start.getTime()));
} catch (Exception e) {
e.printStackTrace();
fail();
}
}
public void testOld() {
try {
Date start = new Date();
System.out.println(Start old);
for (int i=0; i10; i++) {
List newList = new ArrayList();
for (Iterator it = testList.iterator(); it.hasNext();) {
newList.add(((String)it.next()).toUpperCase());
}
assertEquals(((String)newList.get(0)),
((String)testList.get(0)).toUpperCase());
}
System.out.println(End elapsed : [ +(new
Date().getTime()-start.getTime()));
} catch (Exception e) {
e.printStackTrace();
fail();
}
}
public void testMap() {
try {
Date start = new Date();
System.out.println(Start closure);
for (int i=0; i10; i++) {
List newList = ClosureUtils.map(testList, new Closure() {
public Object execute(Object o) {
return ((String)o).toUpperCase();
}
}
);
assertEquals(((String)newList.get(0)),
((String)testList.get(0)).toUpperCase());
}
System.out.println(End elapsed : [ +(new
Date().getTime()-start.getTime()));
} catch
RE: AW: [Patch] modifiedselector, style, remove unused code, slightly more lazy DigestAlgorithm.getValue (now with added source code -doh!)
Just one comment on your ClosureUtils.map() method.
Although this code may be very good for ArrayList, I think
it will behave quite bad for LinkList arguments where
the cost of List.get(i) is quite expensive.
Maybe you should provide for that if you want this as a general
utility pattern.
Jose Alberto
-Original Message-
From: Kev Jackson [mailto:[EMAIL PROTECTED]
Sent: 28 February 2005 07:54
To: Ant Developers List
Subject: Re: AW: [Patch] modifiedselector, style, remove
unused code, slightly more lazy DigestAlgorithm.getValue (now
with added source code -doh!)
Why you use the for-loop instead of iterator?
1 iterator has to check underlying array bounds for every
hasNext(), and
every next(), checking is slow, and in this case unecessary
2 iterator call to hasNext() each time around the loop - again we can
call size() once and cache the result, since we are never
updating the
underlying structure, this won't change and the local var
read is faster
than a method call.
3 iterator is a large object to create, an int is a small one
(bytecode is probably smaller, although I haven't checked this, I can
almost (99%) guarantee it)
4 remove calls to iterator, then we can remove the import :)
It's just an idiom I learnt from Effective Java or one of the books
like that, but the reasons above mean that I use it whenever
I can, as
it does tend to speed up large loops, at the expense of only a very
slight obfuscation of the code. Some people use essentially the same
loop but move the size variable above the loop. That's ok,
but it means
that the size variable has slightly larger scope than it requires,
although the effect is the same.
Also speaking of Iterators in general...
Iterators: Signs of Weakness in Object-Oriented Languages
http://citeseer.ist.psu.edu/55669.html (there's a PDF or PS file
downloadable from the link at the top)
I don't pretend to understand all the Lispness of this paper,
but some
of the arguments against external iterators (the kind we have
in Java)
are interesting. I did a little test last week comparing a normal
iterator/for loop, with a functional-style of programming, both in
Java. Speed was identical, but bytecode of the functional-style was
much smaller, there were less casts and less method calls.
Anecdotal at
best, but try it yourself:
[I decided to call these things Closures, but they aren't
quite, rather
more like Blocks, correct me if I'm getting my terms wrong]
public interface Closure {
Object execute(Object o);
}
import java.util.ArrayList;
import java.util.List;
public class ClosureUtils {
/**
* This method maps a function to a list of data
* and returns a new modified list.
* @param list the source list of data
* @param c the Closure to apply to the list
* @return the modified list
*/
public static List map(final List list, final Closure c) {
final int size = list.size();
List result = new ArrayList(size);
for (int i=0; isize; i++) {
result.add(c.execute(list.get(i)));
}
return result;
}
}
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Date;
import java.util.Iterator;
import java.util.List;
import junit.framework.TestCase;
public class ClosureUtilsTest extends TestCase {
List testList = Arrays.asList(new String[]{This,
That, Other});
/*
* @see TestCase#setUp()
*/
protected void setUp() throws Exception {
super.setUp();
}
/*
* @see TestCase#tearDown()
*/
protected void tearDown() throws Exception {
super.tearDown();
}
public void testJ5() {
try {
Date start = new Date();
System.out.println(Start J5);
for (int i=0; i10; i++) {
List newList = new ArrayList();
for (Object o : testList) {
newList.add(((String)o).toUpperCase());
}
assertEquals(((String)newList.get(0)),
((String)testList.get(0)).toUpperCase());
}
System.out.println(End elapsed : [ +(new
Date().getTime()-start.getTime()));
} catch (Exception e) {
e.printStackTrace();
fail();
}
}
public void testOld() {
try {
Date start = new Date();
System.out.println(Start old);
for (int i=0; i10; i++) {
List newList = new ArrayList();
for (Iterator it = testList.iterator();
it.hasNext();) {
newList.add(((String)it.next()).toUpperCase());
}
assertEquals(((String)newList.get(0)),
((String)testList.get(0)).toUpperCase
Massively OT, 'Closures in Java' [was Re: AW: [Patch] modifiedselector, style, remove unused code, slightly more lazy DigestAlgorithm.getValue (now with added source code -doh!)]
Jose Alberto Fernandez wrote: Just one comment on your ClosureUtils.map() method. Although this code may be very good for ArrayList, I think it will behave quite bad for LinkList arguments where the cost of List.get(i) is quite expensive. When I read this, I thought Doh! you're right none-sequential List access is expensive, but I just rewrote the tests using a LinkedList and the time is identical. Maybe it's because the ClosureUtils requires a List interface not an ArrayList object, and as such it treats everything as simply something that implements List and uses the lowest common denominator, or maybe it's because my test data is trivial [shrug]. Still interesting code-doodle anyway. Thanks for the feedback ;) Kev - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
RE: Massively OT, 'Closures in Java' [was Re: AW: [Patch] modifiedselector, style, remove unused code, slightly more lazy DigestAlgorithm.getValue (now with added source code -doh!)]
AFAIU, LinkedList.get(N) requires traversing the list to position N on every call [O(n^2)], so usage of an iteratoe is much cheaper on this case as there is no array behind the scenes. Jose Alberto -Original Message- From: Kev Jackson [mailto:[EMAIL PROTECTED] Sent: 28 February 2005 11:03 To: Ant Developers List Subject: Massively OT, 'Closures in Java' [was Re: AW: [Patch] modifiedselector, style, remove unused code, slightly more lazy DigestAlgorithm.getValue (now with added source code -doh!)] Jose Alberto Fernandez wrote: Just one comment on your ClosureUtils.map() method. Although this code may be very good for ArrayList, I think it will behave quite bad for LinkList arguments where the cost of List.get(i) is quite expensive. When I read this, I thought Doh! you're right none-sequential List access is expensive, but I just rewrote the tests using a LinkedList and the time is identical. Maybe it's because the ClosureUtils requires a List interface not an ArrayList object, and as such it treats everything as simply something that implements List and uses the lowest common denominator, or maybe it's because my test data is trivial [shrug]. Still interesting code-doodle anyway. Thanks for the feedback ;) Kev - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: Massively OT, 'Closures in Java' [was Re: AW: [Patch] modifiedselector, style, remove unused code, slightly more lazy DigestAlgorithm.getValue (now with added source code -doh!)]
Jose Alberto Fernandez wrote: AFAIU, LinkedList.get(N) requires traversing the list to position N on every call [O(n^2)], so usage of an iteratoe is much cheaper on this case as there is no array behind the scenes. Jose Alberto I've just worked out why it's ok to do it this way (with respect to my particular use-case). Basically I *want* to traverse the entire list, I'm not trying to pick out any particular position in the list (yes truly expensive using LinkedList.get(i)). In my method I simply call get(i) from 0..List.size(): from docs quoteOperations that index into the list will traverse the list from the begining or the end, whichever is closer to the specified index./quote So as I'm asking for position 0, the traversal starts at the head of the list, then I simply walk the list with the loop calling get(i), which also happens to be the next element in the list from where the list cursor currently is (ListIterator interface docs). Serendipity I suppose, but my usage is the most efficient for LinkedLists and also happens to be performant with ArrayLists, to do the particular traversal that I'm doing. Kev - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
RE: Massively OT, 'Closures in Java' [was Re: AW: [Patch] modifiedselector, style, remove unused code, slightly more lazy DigestAlgorithm.getValue (now with added source code -doh!)]
I do not think so, from LinkedList implementation:
/**
* Return the indexed entry.
*/
private Entry entry(int index) {
if (index 0 || index = size)
throw new IndexOutOfBoundsException(Index: +index+
, Size: +size);
Entry e = header;
if (index size/2) {
for (int i = 0; i = index; i++)
e = e.next;
} else {
for (int i = size; i index; i--)
e = e.previous;
}
return e;
}
It will iterate on every call.
Enough of this, as I think it is becoming a little out of subject for
the ANT list :-)
Jose Alberto
-Original Message-
From: Kev Jackson [mailto:[EMAIL PROTECTED]
Sent: 28 February 2005 11:19
To: Ant Developers List
Subject: Re: Massively OT, 'Closures in Java' [was Re: AW:
[Patch] modifiedselector, style, remove unused code, slightly
more lazy DigestAlgorithm.getValue (now with added source code -doh!)]
Jose Alberto Fernandez wrote:
AFAIU, LinkedList.get(N) requires traversing the list to
position N on
every call [O(n^2)], so usage of an iteratoe is much cheaper on this
case as there is no array behind the scenes.
Jose Alberto
I've just worked out why it's ok to do it this way (with
respect to my
particular use-case). Basically I *want* to traverse the
entire list,
I'm not trying to pick out any particular position in the list (yes
truly expensive using LinkedList.get(i)). In my method I simply call
get(i) from 0..List.size():
from docs
quoteOperations that index into the list will traverse the
list from
the begining or the end, whichever is closer to the specified
index./quote
So as I'm asking for position 0, the traversal starts at the
head of the
list, then I simply walk the list with the loop calling get(i), which
also happens to be the next element in the list from where the list
cursor currently is (ListIterator interface docs). Serendipity I
suppose, but my usage is the most efficient for LinkedLists and also
happens to be performant with ArrayLists, to do the
particular traversal
that I'm doing.
Kev
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
RE: Massively OT, 'Closures in Java' [was Re: AW: [Patch] modifiedselector, style, remove unused code, slightly more lazy DigestAlgorithm.getValue (now with added source code -doh!)]
If you want to iterate through a List that has an unspecified
implementation (at the point of iteration) always use an iterator; this
is the only way to implement performant code without making assumptions
about the implementation of the list.
Phil :n.
On Mon, 2005-02-28 at 11:30, Jose Alberto Fernandez wrote:
I do not think so, from LinkedList implementation:
/**
* Return the indexed entry.
*/
private Entry entry(int index) {
if (index 0 || index = size)
throw new IndexOutOfBoundsException(Index: +index+
, Size: +size);
Entry e = header;
if (index size/2) {
for (int i = 0; i = index; i++)
e = e.next;
} else {
for (int i = size; i index; i--)
e = e.previous;
}
return e;
}
It will iterate on every call.
Enough of this, as I think it is becoming a little out of subject for
the ANT list :-)
Jose Alberto
-Original Message-
From: Kev Jackson [mailto:[EMAIL PROTECTED]
Sent: 28 February 2005 11:19
To: Ant Developers List
Subject: Re: Massively OT, 'Closures in Java' [was Re: AW:
[Patch] modifiedselector, style, remove unused code, slightly
more lazy DigestAlgorithm.getValue (now with added source code -doh!)]
Jose Alberto Fernandez wrote:
AFAIU, LinkedList.get(N) requires traversing the list to
position N on
every call [O(n^2)], so usage of an iteratoe is much cheaper on this
case as there is no array behind the scenes.
Jose Alberto
I've just worked out why it's ok to do it this way (with
respect to my
particular use-case). Basically I *want* to traverse the
entire list,
I'm not trying to pick out any particular position in the list (yes
truly expensive using LinkedList.get(i)). In my method I simply call
get(i) from 0..List.size():
from docs
quoteOperations that index into the list will traverse the
list from
the begining or the end, whichever is closer to the specified
index./quote
So as I'm asking for position 0, the traversal starts at the
head of the
list, then I simply walk the list with the loop calling get(i), which
also happens to be the next element in the list from where the list
cursor currently is (ListIterator interface docs). Serendipity I
suppose, but my usage is the most efficient for LinkedLists and also
happens to be performant with ArrayLists, to do the
particular traversal
that I'm doing.
Kev
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
--
Phil Weighill-Smith [EMAIL PROTECTED]
Volantis Systems
