Re: svn commit: r1066963 - in /ant/core/trunk/src/main/org/apache/tools: ant/ ant/taskdefs/ ant/taskdefs/cvslib/ ant/taskdefs/email/ ant/taskdefs/optional/ ant/taskdefs/optional/depend/ ant/taskdefs/o

2011-02-04 Thread Antoine Levy-Lambert 


Hello Stefan,

I did not know that using a final variable for the upper bound of a loop 
instead of something.size() makes a difference in terms of performance. 
Interesting.


I just read the original bug report Project.fireMessageLoggedEvent 
performance fix [1] and the one you have addressed [2].


In an ideal world, should the compiler not do these optimizations for us 
automatically ?


Regards,

Antoine

[1] https://issues.apache.org/bugzilla/show_bug.cgi?id=19101
[2] https://issues.apache.org/bugzilla/show_bug.cgi?id=50716


On 2/3/2011 4:00 PM, bode...@apache.org wrote:

Author: bodewig
Date: Thu Feb  3 21:00:00 2011
New Revision: 1066963

URL: http://svn.apache.org/viewvc?rev=1066963view=rev
Log:
microoptimizations.  PR 50716

Modified:
 ant/core/trunk/src/main/org/apache/tools/ant/IntrospectionHelper.java
 ant/core/trunk/src/main/org/apache/tools/ant/Main.java
 ant/core/trunk/src/main/org/apache/tools/ant/Target.java
 ant/core/trunk/src/main/org/apache/tools/ant/UnknownElement.java

.

URL: 
http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/mail/MailMessage.java?rev=1066963r1=1066962r2=1066963view=diff
==
--- ant/core/trunk/src/main/org/apache/tools/mail/MailMessage.java (original)
+++ ant/core/trunk/src/main/org/apache/tools/mail/MailMessage.java Thu Feb  3 
21:00:00 2011
@@ -328,7 +328,8 @@ public class MailMessage {
  //   Header fields are NOT required to occur in any particular order,
  //except that the message body MUST occur AFTER the headers
  // (the same section specifies a reccommended order, which we ignore)
-   for (int i = 0; i  headersKeys.size(); i++) {
+   final int size = headersKeys.size();
+   for (int i = 0; i  size; i++) {
String name = (String) headersKeys.elementAt(i);
String value = (String) headersValues.elementAt(i);
out.println(name + :  + value);





-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

Re: svn commit: r1066963 - in /ant/core/trunk/src/main/org/apache/tools: ant/ ant/taskdefs/ ant/taskdefs/cvslib/ ant/taskdefs/email/ ant/taskdefs/optional/ ant/taskdefs/optional/depend/ ant/taskdefs/o

2011-02-04 Thread Peter Reilly 
It depends on the cost of .size(), it is done on each run of the body
and cannot be optimized by the compiler (.size() could change during
the loop).

A common pattern is to do:
for (int i = 0, len = x.size(); i  len; ++i) {
   ...
}

Peter


On Fri, Feb 4, 2011 at 2:03 PM, Antoine Levy-Lambert anto...@gmx.de wrote:

 Hello Stefan,

 I did not know that using a final variable for the upper bound of a loop
 instead of something.size() makes a difference in terms of performance.
 Interesting.

 I just read the original bug report Project.fireMessageLoggedEvent
 performance fix [1] and the one you have addressed [2].

 In an ideal world, should the compiler not do these optimizations for us
 automatically ?

 Regards,

 Antoine

 [1] https://issues.apache.org/bugzilla/show_bug.cgi?id=19101
 [2] https://issues.apache.org/bugzilla/show_bug.cgi?id=50716


 On 2/3/2011 4:00 PM, bode...@apache.org wrote:

 Author: bodewig
 Date: Thu Feb  3 21:00:00 2011
 New Revision: 1066963

 URL: http://svn.apache.org/viewvc?rev=1066963view=rev
 Log:
 microoptimizations.  PR 50716

 Modified:
     ant/core/trunk/src/main/org/apache/tools/ant/IntrospectionHelper.java
     ant/core/trunk/src/main/org/apache/tools/ant/Main.java
     ant/core/trunk/src/main/org/apache/tools/ant/Target.java
     ant/core/trunk/src/main/org/apache/tools/ant/UnknownElement.java

 .

 URL:
 http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/mail/MailMessage.java?rev=1066963r1=1066962r2=1066963view=diff

 ==
 --- ant/core/trunk/src/main/org/apache/tools/mail/MailMessage.java
 (original)
 +++ ant/core/trunk/src/main/org/apache/tools/mail/MailMessage.java Thu Feb
  3 21:00:00 2011
 @@ -328,7 +328,8 @@ public class MailMessage {
      //   Header fields are NOT required to occur in any particular
 order,
      //    except that the message body MUST occur AFTER the headers
      // (the same section specifies a reccommended order, which we ignore)
 -   for (int i = 0; i  headersKeys.size(); i++) {
 +   final int size = headersKeys.size();
 +   for (int i = 0; i  size; i++) {
        String name = (String) headersKeys.elementAt(i);
        String value = (String) headersValues.elementAt(i);
        out.println(name + :  + value);




 -
 To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
 For additional commands, e-mail: dev-h...@ant.apache.org



-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

Re: svn commit: r1066963 - in /ant/core/trunk/src/main/org/apache/tools: ant/ ant/taskdefs/ ant/taskdefs/cvslib/ ant/taskdefs/email/ ant/taskdefs/optional/ ant/taskdefs/optional/depend/ ant/taskdefs/o

2011-02-04 Thread Stefan Bodewig 
On 2011-02-04, Antoine Levy-Lambert wrote:

 Hello Stefan,

 I did not know that using a final variable for the upper bound of a
 loop instead of something.size() makes a difference in terms of
 performance. Interesting.

 I just read the original bug report Project.fireMessageLoggedEvent
 performance fix [1] and the one you have addressed [2].

 In an ideal world, should the compiler not do these optimizations for
 us automatically ?

The compiler can not know that size() will always return the same result
during the loop execution - using the external variable makes this
explicit.  So instead of a method call per loop iteration you get a
variable read on a local variable that may even get optimized away as
the variable is known to never change its value.

A smart compiler might see that the variable never gets reassigned so
the final keyword may be redundant.

Most of the places where I have made the changes were one-time
executions or really small loops so the effect is probably close to
zero.

Stefan

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

Re: svn commit: r1066963 - in /ant/core/trunk/src/main/org/apache/tools: ant/ ant/taskdefs/ ant/taskdefs/cvslib/ ant/taskdefs/email/ ant/taskdefs/optional/ ant/taskdefs/optional/depend/ ant/taskdefs/o

2011-02-04 Thread Jesse Glick 

On 02/04/2011 09:03 AM, Antoine Levy-Lambert wrote:

I did not know that using a final variable for the upper bound of a loop 
instead of something.size() makes a difference in terms of performance.


Generally the preferred idiom is to use an iterator, which the Java 5 for loop would do if we were not source=1.4. (In the quoted case you are iterating two lists at 
once so an enhanced for loop would not suffice but Iterator can still be used. In a few other cases the index is needed for another purpose so using an Iterator is less 
desirable.)



-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

Re: svn commit: r1066963 - in /ant/core/trunk/src/main/org/apache/tools: ant/ ant/taskdefs/ ant/taskdefs/cvslib/ ant/taskdefs/email/ ant/taskdefs/optional/ ant/taskdefs/optional/depend/ ant/taskdefs/o

2011-02-04 Thread Stefan Bodewig 
On 2011-02-04, Jesse Glick wrote:

 On 02/04/2011 09:03 AM, Antoine Levy-Lambert wrote:
 I did not know that using a final variable for the upper bound of a loop 
 instead of something.size() makes a difference in terms of performance.

 Generally the preferred idiom is to use an iterator,

Absolutely agreed, but that would have been a bigger change so I shied
away from it (as well as from replacing Vectors with some other lists in
various places).

Stefan

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org