You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2012/09/23 13:37:49 UTC
svn commit: r1389023 -
/tomcat/trunk/java/org/apache/catalina/valves/AccessLogValve.java
Author: markt
Date: Sun Sep 23 11:37:48 2012
New Revision: 1389023
URL: http://svn.apache.org/viewvc?rev=1389023&view=rev
Log:
Review from rjung
Modified:
tomcat/trunk/java/org/apache/catalina/valves/AccessLogValve.java
Modified: tomcat/trunk/java/org/apache/catalina/valves/AccessLogValve.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/valves/AccessLogValve.java?rev=1389023&r1=1389022&r2=1389023&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/valves/AccessLogValve.java (original)
+++ tomcat/trunk/java/org/apache/catalina/valves/AccessLogValve.java Sun Sep 23 11:37:48 2012
@@ -948,7 +948,7 @@ public class AccessLogValve extends Valv
log(result);
// TODO - Make this configurable
- if (result.length() < 256) {
+ if (result.length() <= maxLogMessageBufferSize) {
result.clear();
charBuffers.add(result);
}
@@ -1825,11 +1825,7 @@ public class AccessLogValve extends Valv
// second
buf.append(Long.toString(time / 1000));
buf.append('.');
- int remains = (int) (time % 1000);
- buf.append(Long.toString(remains / 100));
- remains = remains % 100;
- buf.append(Long.toString(remains / 10));
- buf.append(Long.toString(remains % 10));
+ buf.append(Long.toString(time % 1000));
}
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1389023 - /tomcat/trunk/java/org/apache/catalina/valves/AccessLogValve.java
Posted by Mark Thomas <ma...@apache.org>.
On 23/09/2012 15:10, Rainer Jung wrote:
> On 23.09.2012 14:56, Konstantin Kolinko wrote:
>> 2012/9/23 <ma...@apache.org>:
>>> Author: markt
>>> Date: Sun Sep 23 11:37:48 2012
>>> New Revision: 1389023
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1389023&view=rev
>>> Log:
>>> Review from rjung
>>>
>>> Modified:
>>> tomcat/trunk/java/org/apache/catalina/valves/AccessLogValve.java
>>>
>>> Modified:
>>> tomcat/trunk/java/org/apache/catalina/valves/AccessLogValve.java
>>> URL:
>>> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/valves/AccessLogValve.java?rev=1389023&r1=1389022&r2=1389023&view=diff
>>>
>>> ==============================================================================
>>>
>>> --- tomcat/trunk/java/org/apache/catalina/valves/AccessLogValve.java
>>> (original)
>>> +++ tomcat/trunk/java/org/apache/catalina/valves/AccessLogValve.java
>>> Sun Sep 23 11:37:48 2012
>>> @@ -948,7 +948,7 @@ public class AccessLogValve extends Valv
>>> log(result);
>>>
>>> // TODO - Make this configurable
>>> - if (result.length() < 256) {
>>> + if (result.length() <= maxLogMessageBufferSize) {
>>> result.clear();
>>> charBuffers.add(result);
>>> }
>>> @@ -1825,11 +1825,7 @@ public class AccessLogValve extends Valv
>>> // second
>>> buf.append(Long.toString(time / 1000));
>>> buf.append('.');
>>> - int remains = (int) (time % 1000);
>>> - buf.append(Long.toString(remains / 100));
>>> - remains = remains % 100;
>>> - buf.append(Long.toString(remains / 10));
>>> - buf.append(Long.toString(remains % 10));
>>> + buf.append(Long.toString(time % 1000));
>>
>> The above change is plainly wrong. It loses leading zeros in the
>> fractional part.
>> E.g. it'll print 1060 msec as "1.60" sec (1600).
>>
>>> }
>>> }
>>> }
>
> Oups, sorry for my misleading advice.
>
> Will
>
> // Format modulo 1000 and include leading zeroes
> buf.append(Long.toString(time % 1000 + 10000).substring(1));
>
> be still better than the original code? I think using a formatter is
> overkill for this simple king of leading zero's formatting and calling
> append plus Long.toString() three times might still be worse than an
> additional substring() which needs to allocate a new String object but
> will share the underlying char array.
>
> Alternatively one could use an if-then-else chain depending on the size
> of time%1000 and appending nothing, "0", "00" or "000" before appending
> the Long.toString(time % 1000).
I'll restore the original and not worry about it until it crops up in
the profiling results (if it crops up at all).
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1389023 - /tomcat/trunk/java/org/apache/catalina/valves/AccessLogValve.java
Posted by Rainer Jung <ra...@kippdata.de>.
On 23.09.2012 14:56, Konstantin Kolinko wrote:
> 2012/9/23 <ma...@apache.org>:
>> Author: markt
>> Date: Sun Sep 23 11:37:48 2012
>> New Revision: 1389023
>>
>> URL: http://svn.apache.org/viewvc?rev=1389023&view=rev
>> Log:
>> Review from rjung
>>
>> Modified:
>> tomcat/trunk/java/org/apache/catalina/valves/AccessLogValve.java
>>
>> Modified: tomcat/trunk/java/org/apache/catalina/valves/AccessLogValve.java
>> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/valves/AccessLogValve.java?rev=1389023&r1=1389022&r2=1389023&view=diff
>> ==============================================================================
>> --- tomcat/trunk/java/org/apache/catalina/valves/AccessLogValve.java (original)
>> +++ tomcat/trunk/java/org/apache/catalina/valves/AccessLogValve.java Sun Sep 23 11:37:48 2012
>> @@ -948,7 +948,7 @@ public class AccessLogValve extends Valv
>> log(result);
>>
>> // TODO - Make this configurable
>> - if (result.length() < 256) {
>> + if (result.length() <= maxLogMessageBufferSize) {
>> result.clear();
>> charBuffers.add(result);
>> }
>> @@ -1825,11 +1825,7 @@ public class AccessLogValve extends Valv
>> // second
>> buf.append(Long.toString(time / 1000));
>> buf.append('.');
>> - int remains = (int) (time % 1000);
>> - buf.append(Long.toString(remains / 100));
>> - remains = remains % 100;
>> - buf.append(Long.toString(remains / 10));
>> - buf.append(Long.toString(remains % 10));
>> + buf.append(Long.toString(time % 1000));
>
> The above change is plainly wrong. It loses leading zeros in the
> fractional part.
> E.g. it'll print 1060 msec as "1.60" sec (1600).
>
>> }
>> }
>> }
Oups, sorry for my misleading advice.
Will
// Format modulo 1000 and include leading zeroes
buf.append(Long.toString(time % 1000 + 10000).substring(1));
be still better than the original code? I think using a formatter is
overkill for this simple king of leading zero's formatting and calling
append plus Long.toString() three times might still be worse than an
additional substring() which needs to allocate a new String object but
will share the underlying char array.
Alternatively one could use an if-then-else chain depending on the size
of time%1000 and appending nothing, "0", "00" or "000" before appending
the Long.toString(time % 1000).
Regards,
Rainer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1389023 - /tomcat/trunk/java/org/apache/catalina/valves/AccessLogValve.java
Posted by Konstantin Kolinko <kn...@gmail.com>.
2012/9/23 <ma...@apache.org>:
> Author: markt
> Date: Sun Sep 23 11:37:48 2012
> New Revision: 1389023
>
> URL: http://svn.apache.org/viewvc?rev=1389023&view=rev
> Log:
> Review from rjung
>
> Modified:
> tomcat/trunk/java/org/apache/catalina/valves/AccessLogValve.java
>
> Modified: tomcat/trunk/java/org/apache/catalina/valves/AccessLogValve.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/valves/AccessLogValve.java?rev=1389023&r1=1389022&r2=1389023&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/valves/AccessLogValve.java (original)
> +++ tomcat/trunk/java/org/apache/catalina/valves/AccessLogValve.java Sun Sep 23 11:37:48 2012
> @@ -948,7 +948,7 @@ public class AccessLogValve extends Valv
> log(result);
>
> // TODO - Make this configurable
> - if (result.length() < 256) {
> + if (result.length() <= maxLogMessageBufferSize) {
> result.clear();
> charBuffers.add(result);
> }
> @@ -1825,11 +1825,7 @@ public class AccessLogValve extends Valv
> // second
> buf.append(Long.toString(time / 1000));
> buf.append('.');
> - int remains = (int) (time % 1000);
> - buf.append(Long.toString(remains / 100));
> - remains = remains % 100;
> - buf.append(Long.toString(remains / 10));
> - buf.append(Long.toString(remains % 10));
> + buf.append(Long.toString(time % 1000));
The above change is plainly wrong. It loses leading zeros in the
fractional part.
E.g. it'll print 1060 msec as "1.60" sec (1600).
> }
> }
> }
>
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org