You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by Joachim Draeger <jd...@gmx.de> on 2006/11/14 08:46:53 UTC
isLogEnabled() and StringBuffer
Hi!
Am I hitting another controversy topic? ;-)
Example from ImapHandler.java
catch ( IOException e ) {
if ( getLogger().isErrorEnabled() ) {
StringBuffer exceptionBuffer =
new StringBuffer( 256 )
.append( "Cannot open connection from " )
.append( remoteHost )
.append( " (" )
.append( remoteIP )
.append( "): " )
.append( e.getMessage() );
getLogger().error( exceptionBuffer.toString(), e );
}
throw e;
}
IMHO this blows up the code and reduces the readability by only bringing
a negligible performance enhancement.
IMO the bottleneck is IO and in the big frameworks (JavaMail ;-) and not
in simple string computation.
Better readability will bring our users less bugs and costs only a few
microseconds.
IMO StringBuffer is only mandatory when dealing with really big Strings
(like message headers/body) or when doing really much concats.
I would use isDebugEnabled() only inside a loop and not in a per command
issue.
BTW:
catch ( IOException e ) {
getLogger().error( "Cannot open connection from " + remoteHost + " ("
+ remoteIP + "): " + e.getMessage() );
throw e;
}
To make it clear: I'm not proposing refactoring all around James. I just
don't want to make extensive use of isLogEnabled() and StringBuffer().
I'm going to remove it from Imap code where I think it brings more
readability.
WDYT?
Joachim
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: isLogEnabled() and StringBuffer
Posted by Stefano Bagnara <ap...@bago.org>.
Bernd Fondermann wrote:
> On 11/17/06, Stefano Bagnara <ap...@bago.org> wrote:
>> ----
>> Output code 2 from jdk 1.4
>> > String s2 = s + "test" + s1 + "test" + i;
>> ----
>> Output code 2 from jdk 1.5
>> > String s2 = (new StringBuilder()).append(s).append("test")
>> > .append(s1).append("test").append(i).toString();
>>
>> I also ran with jdk5 that code in a loop of 1 million iterations using
>> foo = "foo" and bar = "bar" (this is FAR from any realistic scenario).
>
> <sigh> Microbenchmarking, again! ;-)
> Could it be you are testing loop optimization here? ;-)
I started the message with a worning about this ;-)
Anyway I did a lot of micro changes to try to understand wether the
microtest was realistic or not. As an example if I concatenate 2 more
tokens in the same line the resulting time is increased by 30%. I also
increased and decreased the loop count and randomly generated the
contents of the variables. Times was always comparable: +/- 20%
> What also is totally obscured by the whole discussion, is to set in
> relation the max performance difference of the different logging
> implementations discussed here - the "delta" - to the cost of the
> rest of the method. Only if delta is a significant cost (a "hot spot")
> it is worth talking about optimizing it before optimizing everything
> else.
>
> Bernd
I agree on this.
I just wanted to show more detail about the "old good practices" we all
know vs new jvm/jdk.
Imho it is really interesting to see that in jdk 1.5 the string
concatenation is replaced by StringBuilder by default and this way is
even better than using the StringBuffer.
To me this mean: using StringBuilder make my code to require java 5,
using StringBuffer make my code to work better on java 2 1.4, worse on
java 5, using String concatenation make my code to work best if I
compile/run with java 5, otherwise it perform a little worse, but very
little in java 2 1.4+.
I've not found papers/test on the web about this issue, I did it, I had
an unexpected result. Maybe you all know about this optimizations in
java5, but now I will use pure string concatenation much more.
Stefano
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: isLogEnabled() and StringBuffer
Posted by Joachim Draeger <jd...@gmx.de>.
Hi Bernd,
Am Freitag, den 17.11.2006, 08:42 +0100 schrieb Bernd Fondermann:
> > I also ran with jdk5 that code in a loop of 1 million iterations using
> > foo = "foo" and bar = "bar" (this is FAR from any realistic scenario).
>
> <sigh> Microbenchmarking, again! ;-)
> Could it be you are testing loop optimization here? ;-)
>
> What also is totally obscured by the whole discussion, is to set in
> relation the max performance difference of the different logging
> implementations discussed here - the "delta" - to the cost of the
> rest of the method. Only if delta is a significant cost (a "hot spot")
> it is worth talking about optimizing it before optimizing everything
> else.
+1
This is exactly my thought. As long as nobody showed me (even by a
extrapolation) that its costs are significant, I consider even those
is<Log>Enabled on a per command issue as programmers voodoo and rumors.
IMO the users will benefit more from readable, maintainable code than
from 0.5 % memory savings and 0.5 % more concurrent connections.
(replace the numbers with your profiling results)
BTW: I have often seen NPE *bugs* in debug messages, that appeared
magically when the costumer has turned on *de*bugging. :-) This could
avoided by putting a try/catch block around...
Joachim
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: isLogEnabled() and StringBuffer
Posted by Vincenzo Gianferrari Pini <vi...@praxis.it>.
:-)
Vincenzo
Joachim Draeger wrote:
> Vincenzo Gianferrari Pini wrote:
>>>> BTW: I have often seen NPE *bugs* in debug messages, that appeared
>>>> magically when the costumer has turned on *de*bugging. :-) This could
>>>> avoided by putting a try/catch block around...
>>>>
>>>>
>>> Im against this.. Even if the NPE is in the debug message it should not
>>> catched. IMHO thats a bad practice.. NPE should fixed not "catched" ;-)
>>>
>> Norman is totally right.
>>
>> We should notice that the NullPointerException class is on purpose a
>> subclass of RuntimeException, whose handling is not enforced by the
>> java compiler just because it is a "coding error" that, by
>> definition, can not be anticipated and should just be fixed when
>> found. It does not make sense to anticipate any possible
>> RuntimeException *locally* in any possible place surrounding
>> everything everywhere by try/catch blocks.
>
> Sorry! I should have written:
> <irony> This could avoided by putting a try/catch block around...
> </irony>
> Shame on me, I know it is bad style to use irony 1. in electronic
> communication 2. in a language that is not ones first.
> I was joking related to methods to blow up code.
>
> Joachim
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: isLogEnabled() and StringBuffer
Posted by Joachim Draeger <jd...@joachim-draeger.de>.
Vincenzo Gianferrari Pini wrote:
>>> BTW: I have often seen NPE *bugs* in debug messages, that appeared
>>> magically when the costumer has turned on *de*bugging. :-) This could
>>> avoided by putting a try/catch block around...
>>>
>>>
>> Im against this.. Even if the NPE is in the debug message it should not
>> catched. IMHO thats a bad practice.. NPE should fixed not "catched" ;-)
>>
> Norman is totally right.
>
> We should notice that the NullPointerException class is on purpose a
> subclass of RuntimeException, whose handling is not enforced by the
> java compiler just because it is a "coding error" that, by
> definition, can not be anticipated and should just be fixed when
> found. It does not make sense to anticipate any possible
> RuntimeException *locally* in any possible place surrounding
> everything everywhere by try/catch blocks.
Sorry! I should have written:
<irony> This could avoided by putting a try/catch block around... </irony>
Shame on me, I know it is bad style to use irony 1. in electronic
communication 2. in a language that is not ones first.
I was joking related to methods to blow up code.
Joachim
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: isLogEnabled() and StringBuffer
Posted by Vincenzo Gianferrari Pini <vi...@praxis.it>.
Norman Maurer wrote:
> Joachim Draeger schrieb:
>
>
...
>
>> BTW: I have often seen NPE *bugs* in debug messages, that appeared
>> magically when the costumer has turned on *de*bugging. :-) This could
>> avoided by putting a try/catch block around...
>>
>> Joachim
>>
>>
>>
> Im against this.. Even if the NPE is in the debug message it should not
> catched. IMHO thats a bad practice.. NPE should fixed not "catched" ;-)
>
> bye
> Norman
>
>
>
>
Norman is totally right.
We should notice that the NullPointerException class is on purpose a
subclass of RuntimeException, whose handling is not enforced by the java
compiler just because it is a "coding error" that, by definition, can
not be anticipated and should just be fixed when found. It does not make
sense to anticipate any possible RuntimeException *locally* in any
possible place surrounding everything everywhere by try/catch blocks.
It does make sense instead to have a global try/catch block somewhere to
avoid having a server crash, and this does exist in James (see
http://wiki.apache.org/james/HandlingExceptions).
Vincenzo
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: isLogEnabled() and StringBuffer
Posted by Norman Maurer <nm...@byteaction.de>.
Joachim Draeger schrieb:
> Hi Bernd,
>
> Am Freitag, den 17.11.2006, 08:42 +0100 schrieb Bernd Fondermann:
>
>
>>> I also ran with jdk5 that code in a loop of 1 million iterations using
>>> foo = "foo" and bar = "bar" (this is FAR from any realistic scenario).
>>>
>> <sigh> Microbenchmarking, again! ;-)
>> Could it be you are testing loop optimization here? ;-)
>>
>> What also is totally obscured by the whole discussion, is to set in
>> relation the max performance difference of the different logging
>> implementations discussed here - the "delta" - to the cost of the
>> rest of the method. Only if delta is a significant cost (a "hot spot")
>> it is worth talking about optimizing it before optimizing everything
>> else.
>>
>
> +1
>
> This is exactly my thought. As long as nobody showed me (even by a
> extrapolation) that its costs are significant, I consider even those
> is<Log>Enabled on a per command issue as programmers voodoo and rumors.
>
> IMO the users will benefit more from readable, maintainable code than
> from 0.5 % memory savings and 0.5 % more concurrent connections.
> (replace the numbers with your profiling results)
>
>
+1
> BTW: I have often seen NPE *bugs* in debug messages, that appeared
> magically when the costumer has turned on *de*bugging. :-) This could
> avoided by putting a try/catch block around...
>
> Joachim
>
>
Im against this.. Even if the NPE is in the debug message it should not
catched. IMHO thats a bad practice.. NPE should fixed not "catched" ;-)
bye
Norman
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: isLogEnabled() and StringBuffer
Posted by Bernd Fondermann <be...@googlemail.com>.
On 11/17/06, Stefano Bagnara <ap...@bago.org> wrote:
> ----
> Output code 2 from jdk 1.4
> > String s2 = s + "test" + s1 + "test" + i;
> ----
> Output code 2 from jdk 1.5
> > String s2 = (new StringBuilder()).append(s).append("test")
> > .append(s1).append("test").append(i).toString();
>
> I also ran with jdk5 that code in a loop of 1 million iterations using
> foo = "foo" and bar = "bar" (this is FAR from any realistic scenario).
<sigh> Microbenchmarking, again! ;-)
Could it be you are testing loop optimization here? ;-)
What also is totally obscured by the whole discussion, is to set in
relation the max performance difference of the different logging
implementations discussed here - the "delta" - to the cost of the
rest of the method. Only if delta is a significant cost (a "hot spot")
it is worth talking about optimizing it before optimizing everything
else.
Bernd
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
RE: isLogEnabled() and StringBuffer
Posted by "Noel J. Bergman" <no...@devtech.com>.
Stefano Bagnara wrote:
> Input code 2:
> String res = foo + "test" + bar + "test" + i;
> Output code 2 from jdk 1.4
> String s2 = s + "test" + s1 + "test" + i;
> Output code 2 from jdk 1.5
> String s2 = (new StringBuilder()).append(s).append("test")
> .append(s1).append("test").append(i).toString();
> I also tried using jdk1.5 using target 1.4 and the resulting class equal
> to the jdk1.4 generated class.
No surprise there. StringBuilder is new in 1.5.0.
> if we compile with jdk 1.5 it is better to use real string concatenation
> even when we have variables and not only literals.
Perhaps.
> It seems that using jdk1.4 results are comparable
How do you come to that conclusion? I don't agree.
--- Noel
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: isLogEnabled() and StringBuffer
Posted by Stefano Bagnara <ap...@bago.org>.
robert burrell donkin wrote:
> On 11/14/06, Stefano Bagnara <ap...@bago.org> wrote:
>> Maybe this helps on this topic:
>> http://wiki.java.net/bin/view/Javapedia/AlwaysUseStringBufferMisconception#Concatenating_Literal_Strings
>
> note that the misconception the link talks about is concatinating
> string literals, not variables
>
> use of string buffers will be quicker and use less memory than
> explicit string assignments when doing variable composition (as is
> typically the case when logging)
>
> - robert
I took 5 minutes to do a synthetic test about this issue: I'm aware that
synthetic tests have to analyzed carefully, but this one seems to be
really simple. bar and foo are 2 String variable defined in the code
just before. They are *not* final.
Input code 1:
> StringBuffer resb = new StringBuffer();
> resb.append(foo).append("test").append(bar).append("test").append(i);
> String res = resb.toString();
----
Input code 2:
> String res = foo + "test" + bar + "test" + i;
----
Output code 1 from eclipse 3.2.1
> StringBuffer resb = new StringBuffer();
> resb.append(foo).append("test").append(bar).append("test").append(i);
> String s = resb.toString();
----
Output code 1 from sun jdk 1.4
> StringBuffer stringbuffer = new StringBuffer();
> stringbuffer.append(s).append("test").append(s1)
> .append("test").append(i);
> String s2 = stringbuffer.toString();
----
Output code 1 from sun jdk 5
> StringBuffer stringbuffer = new StringBuffer();
> stringbuffer.append(s).append("test").append(s1)
> .append("test").append(i);
> String s2 = stringbuffer.toString();
----
Output code 2 from eclipse 3.2.1
> String s = foo + "test" + bar + "test" + i;
----
Output code 2 from jdk 1.4
> String s2 = s + "test" + s1 + "test" + i;
----
Output code 2 from jdk 1.5
> String s2 = (new StringBuilder()).append(s).append("test")
> .append(s1).append("test").append(i).toString();
I also ran with jdk5 that code in a loop of 1 million iterations using
foo = "foo" and bar = "bar" (this is FAR from any realistic scenario).
JRE 1.5.0 Windows
Test1_*.class => 850 - 1020 ms
Test2_eclipse321.class => 880 - 1010 ms
Test2_sunjavac14.class => 830 - 1010 ms
Test2_sunjavac15.class => 780 - 890 ms
JRE 1.4.2 Windows
Test1_sunjavac14.class => 1080 - 1210 ms
Test2_sunjavac14.class => 1050 - 1250 ms
I also tried using jdk1.5 using target 1.4 and the resulting class equal
sto the jdk1.4 generated class.
This is really a simple test, but my conclusion is that if we compile
with jdk 1.5 it is better to use real string concatenation even when we
have variables and not only literals.
It is interesting that using jdk1.5 to compile it will automatically use
a StringBuilder instead of the StringBuffer.
Either way it seems that given the timings at least in a synthetic test
the jvm will optimize the loop even when the code has been compiled as a
simple string concatenation.
After all I think we should keep using the getLogger().isXXXXEnabled()
but we could start using direct string concatenation when we can do this
in a single line. It seems that using jdk1.4 results are comparable
while using jdk1.5 it is even better to use the direct string
concatenation. If we add that direct string concatenation is so much
better to read....
Stefano
PS: it took much more time to write this mail than to write the tests ;-)
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: isLogEnabled() and StringBuffer
Posted by robert burrell donkin <ro...@gmail.com>.
On 11/14/06, Stefano Bagnara <ap...@bago.org> wrote:
> Maybe this helps on this topic:
> http://wiki.java.net/bin/view/Javapedia/AlwaysUseStringBufferMisconception#Concatenating_Literal_Strings
note that the misconception the link talks about is concatinating
string literals, not variables
use of string buffers will be quicker and use less memory than
explicit string assignments when doing variable composition (as is
typically the case when logging)
- robert
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: isLogEnabled() and StringBuffer
Posted by Bernd Fondermann <be...@googlemail.com>.
:-) thanks, interesting read.
although I must admit that my memory/GC statement indeed was a bit
sloppy and is not covered here, AFAICS.
Bernd
On 11/14/06, Stefano Bagnara <ap...@bago.org> wrote:
> Maybe this helps on this topic:
> http://wiki.java.net/bin/view/Javapedia/AlwaysUseStringBufferMisconception#Concatenating_Literal_Strings
>
> Stefano
>
> Danny Angus wrote:
> > On 11/14/06, Bernd Fondermann <be...@googlemail.com> wrote:
> >> On 11/14/06, Joachim Draeger <jd...@gmx.de> wrote:
> >
> >> Memory garbage should not be a problem any longer in the light of
> >> generational GCs.
> >
> > I'm not sure how you arrive at this sweeping conclusion!
> > Profligate use of memory is a problem no matter what CG algorythm you
> > use because even if the collector is capable of collecting it uses
> > clock cycles to do it. This is a cost. If your application is reaching
> > the bounds of its resources, because it is under heavy load, then
> > unecessary use of cpu or memory will be a factor in the limits of
> > performance which you can reach.
> >
> > A large number of string concatenations will always result in a lot of
> > allocations, even if it is optimised to use StringBuffer, unless you
> > are using StringBuffer directly you will have a string (or byte[] or
> > whatever) as input for each component part, a string buffer which is
> > larger than the output and an output string. Thus you use 200% more
> > memory than not doing it at all.
> >
> > d.
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: isLogEnabled() and StringBuffer
Posted by Stefano Bagnara <ap...@bago.org>.
Maybe this helps on this topic:
http://wiki.java.net/bin/view/Javapedia/AlwaysUseStringBufferMisconception#Concatenating_Literal_Strings
Stefano
Danny Angus wrote:
> On 11/14/06, Bernd Fondermann <be...@googlemail.com> wrote:
>> On 11/14/06, Joachim Draeger <jd...@gmx.de> wrote:
>
>> Memory garbage should not be a problem any longer in the light of
>> generational GCs.
>
> I'm not sure how you arrive at this sweeping conclusion!
> Profligate use of memory is a problem no matter what CG algorythm you
> use because even if the collector is capable of collecting it uses
> clock cycles to do it. This is a cost. If your application is reaching
> the bounds of its resources, because it is under heavy load, then
> unecessary use of cpu or memory will be a factor in the limits of
> performance which you can reach.
>
> A large number of string concatenations will always result in a lot of
> allocations, even if it is optimised to use StringBuffer, unless you
> are using StringBuffer directly you will have a string (or byte[] or
> whatever) as input for each component part, a string buffer which is
> larger than the output and an output string. Thus you use 200% more
> memory than not doing it at all.
>
> d.
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: isLogEnabled() and StringBuffer
Posted by Danny Angus <da...@gmail.com>.
On 11/14/06, Bernd Fondermann <be...@googlemail.com> wrote:
> On 11/14/06, Joachim Draeger <jd...@gmx.de> wrote:
> Memory garbage should not be a problem any longer in the light of
> generational GCs.
I'm not sure how you arrive at this sweeping conclusion!
Profligate use of memory is a problem no matter what CG algorythm you
use because even if the collector is capable of collecting it uses
clock cycles to do it. This is a cost. If your application is reaching
the bounds of its resources, because it is under heavy load, then
unecessary use of cpu or memory will be a factor in the limits of
performance which you can reach.
A large number of string concatenations will always result in a lot of
allocations, even if it is optimised to use StringBuffer, unless you
are using StringBuffer directly you will have a string (or byte[] or
whatever) as input for each component part, a string buffer which is
larger than the output and an output string. Thus you use 200% more
memory than not doing it at all.
d.
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: isLogEnabled() and StringBuffer
Posted by robert burrell donkin <ro...@gmail.com>.
On 11/14/06, Bernd Fondermann <be...@googlemail.com> wrote:
> On 11/14/06, Joachim Draeger <jd...@gmx.de> wrote:
> >
> > Hi!
> >
> > Am I hitting another controversy topic? ;-)
>
> We will see ;-)
>
> >
> > Example from ImapHandler.java
> >
> > catch ( IOException e ) {
> > if ( getLogger().isErrorEnabled() ) {
> > StringBuffer exceptionBuffer =
> > new StringBuffer( 256 )
> > .append( "Cannot open connection from " )
> > .append( remoteHost )
> > .append( " (" )
> > .append( remoteIP )
> > .append( "): " )
> > .append( e.getMessage() );
> > getLogger().error( exceptionBuffer.toString(), e );
> > }
> > throw e;
> > }
> >
> > IMHO this blows up the code and reduces the readability by only bringing
> > a negligible performance enhancement.
> > IMO the bottleneck is IO and in the big frameworks (JavaMail ;-) and not
> > in simple string computation.
> >
> > Better readability will bring our users less bugs and costs only a few
> > microseconds.
> >
> > IMO StringBuffer is only mandatory when dealing with really big Strings
> > (like message headers/body) or when doing really much concats.
> > I would use isDebugEnabled() only inside a loop and not in a per command
> > issue.
> > To make it clear: I'm not proposing refactoring all around James. I just
> > don't want to make extensive use of isLogEnabled() and StringBuffer().
> > I'm going to remove it from Imap code where I think it brings more
> > readability.
> >
> > WDYT?
>
> AFAIK, String concatenations on a single line (not in loops or more
> complex situations) are optimized by the compiler to actually use
> StringBuffer in byte code.
the problem has always been that the optimization used by most
compilers is inefficient and results in lots of temporary strings
being created. string objects are a special case and have a habit of
hanging around in memory.
> I also agree that the performance gain can be neglected.
if may sometimes be negligable but in a well optimized application, it
usual is not
> Memory garbage should not be a problem any longer in the light of
> generational GCs.
that is not necessarily true: strings are treated as a special case by
garbage collectors
- robert
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: isLogEnabled() and StringBuffer
Posted by Bernd Fondermann <be...@googlemail.com>.
On 11/14/06, Joachim Draeger <jd...@gmx.de> wrote:
>
> Hi!
>
> Am I hitting another controversy topic? ;-)
We will see ;-)
>
> Example from ImapHandler.java
>
> catch ( IOException e ) {
> if ( getLogger().isErrorEnabled() ) {
> StringBuffer exceptionBuffer =
> new StringBuffer( 256 )
> .append( "Cannot open connection from " )
> .append( remoteHost )
> .append( " (" )
> .append( remoteIP )
> .append( "): " )
> .append( e.getMessage() );
> getLogger().error( exceptionBuffer.toString(), e );
> }
> throw e;
> }
>
> IMHO this blows up the code and reduces the readability by only bringing
> a negligible performance enhancement.
> IMO the bottleneck is IO and in the big frameworks (JavaMail ;-) and not
> in simple string computation.
>
> Better readability will bring our users less bugs and costs only a few
> microseconds.
>
> IMO StringBuffer is only mandatory when dealing with really big Strings
> (like message headers/body) or when doing really much concats.
> I would use isDebugEnabled() only inside a loop and not in a per command
> issue.
> To make it clear: I'm not proposing refactoring all around James. I just
> don't want to make extensive use of isLogEnabled() and StringBuffer().
> I'm going to remove it from Imap code where I think it brings more
> readability.
>
> WDYT?
AFAIK, String concatenations on a single line (not in loops or more
complex situations) are optimized by the compiler to actually use
StringBuffer in byte code.
I also agree that the performance gain can be neglected.
Memory garbage should not be a problem any longer in the light of
generational GCs.
So, I would support to simplify this in new code.
Bernd
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: isLogEnabled() and StringBuffer
Posted by robert burrell donkin <ro...@gmail.com>.
On 11/14/06, Joachim Draeger <jd...@gmx.de> wrote:
>
> Hi!
>
> Am I hitting another controversy topic? ;-)
>
> Example from ImapHandler.java
>
> catch ( IOException e ) {
> if ( getLogger().isErrorEnabled() ) {
> StringBuffer exceptionBuffer =
> new StringBuffer( 256 )
> .append( "Cannot open connection from " )
> .append( remoteHost )
> .append( " (" )
> .append( remoteIP )
> .append( "): " )
> .append( e.getMessage() );
> getLogger().error( exceptionBuffer.toString(), e );
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
IMHO (if avalon log accepts objects) this code should be:
getLogger().error( exceptionBuffer, e );
so that the toString is not resolved until needed
FWIW i would also personally factor the logging into a separate
private method with a meaningful name to improve readability
- robert
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
RE: isLogEnabled() and StringBuffer
Posted by "Noel J. Bergman" <no...@devtech.com>.
The path that you suggest is the exact opposite of a code review and code
change that we deliberately undertook after performance testing.
is<LogLevel>Enabled should be used before formatting the message, and
StringBuffers is more efficient than String concatentation at very little
cost.
We can review to see if the StringBuffer case still applies, but checking
the loglevel is still appropriate.
--- Noel
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org