You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Alexander Shirkov <sg...@gmail.com> on 2010/07/15 06:11:22 UTC

Bug 49165 - Allow %{TIME_FORMAT}t As Configuration for AccessLogValve

Hello,

I¹m working on Bug 49164 (
https://issues.apache.org/bugzilla/show_bug.cgi?id=49165)

Have a question: 
Any hidden reasons, why following construction was used for creating %t
pattern?:

private SimpleDateFormat dayFormatter = new SimpleDateFormat("dd");
private SimpleDateFormat monthFormatter = new SimpleDateFormat("MM");
private SimpleDateFormat yearFormatter = new SimpleDateFormat("yyyy");
private SimpleDateFormat timeFormatter = new SimpleDateFormat("HH:mm:ss");
...
StringBuilder current = new StringBuilder(32);
current.append('[');
current.append(struct.dayFormatter.format(date));
current.append('/');
current.append(lookup(struct.monthFormatter.format(date)));
current.append('/');
current.append(struct.yearFormatter.format(date));
current.append(':');
current.append(struct.timeFormatter.format(date));
current.append(' ');
current.append(getTimeZone(date));
current.append(']');
struct.currentDateString = current.toString();

Instead of this one:

private SimpleDateFormat timeFormatter = new
SimpleDateFormat("[dd/MMM/yyyy:HH:mm:ss]");
...
struct.currentDateString = struct.timeFormatter.format(date);

I¹ve added one more test with this structure to
org.apache.catalina.valves.Benchmarks and looks like the last solution the
shortest and fastest option. Plus we can pass any pattern from configuration
(#49165 enhancement intention):

TimeDateElementBenchmarkTest_Sync: 5 threads and 10000000 iterations using
Syncs took 3392ms
TimeDateElementBenchmarkTest_Local: 5 threads and 10000000 iterations using
ThreadLocals took 2551ms
TimeDateElementBenchmarkTest_LocalStruct: 5 threads and 10000000 iterations
using single ThreadLocal took 2237ms
TimeDateElementBenchmarkTest_LocalStruct_SBuilder: 5 threads and 10000000
iterations using single ThreadLocal, with StringBuilder took 2196ms
TimeDateElementBenchmarkTest_LocalStruct_SimpleDateFormat: 5 threads and
10000000 iterations using single ThreadLocal SimpleDateFormat took 2152ms
 

P. S. My thanks to Mark Thomas for buglist to start.



Re: Bug 49165 - Allow %{TIME_FORMAT}t As Configuration for AccessLogValve

Posted by Konstantin Kolinko <kn...@gmail.com>.
2010/7/17 Alexander Shirkov <sg...@gmail.com>:
> I've submitted patch and related unit tests.
>
> Some questions about the process:
> - should I assign bug to myself?

No. It will prevent bug update notifications from being sent to the dev@ list.

> - should I change bug status from NEW to RESOLVED after submitting the
> patch?

No.  A bug is marked as resolved only after the issue was fixed in all
affected Tomcat versions.

Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Bug 49165 - Allow %{TIME_FORMAT}t As Configuration for AccessLogValve

Posted by Alexander Shirkov <sg...@gmail.com>.
I've submitted patch and related unit tests.

Some questions about the process:
- should I assign bug to myself?
- should I change bug status from NEW to RESOLVED after submitting the
patch?

Thanks,
Alexander

On 17.07.10 6:28, "Mark Thomas" <ma...@apache.org> wrote:

> On 15/07/2010 05:11, Alexander Shirkov wrote:
>> Hello,
>> 
>> I¹m working on Bug 49164 (
>> https://issues.apache.org/bugzilla/show_bug.cgi?id=49165)
>> 
>> Have a question:
>> Any hidden reasons, why following construction was used for creating %t
>> pattern?:
> 
> It stems from this:
> http://svn.apache.org/viewvc?view=revision&revision=285381
> 
> It appears to be performance related but from the figures you present
> below, it may now be unnecessary.
> 
> Mark
> 
>> 
>> private SimpleDateFormat dayFormatter = new SimpleDateFormat("dd");
>> private SimpleDateFormat monthFormatter = new SimpleDateFormat("MM");
>> private SimpleDateFormat yearFormatter = new SimpleDateFormat("yyyy");
>> private SimpleDateFormat timeFormatter = new SimpleDateFormat("HH:mm:ss");
>> ...
>> StringBuilder current = new StringBuilder(32);
>> current.append('[');
>> current.append(struct.dayFormatter.format(date));
>> current.append('/');
>> current.append(lookup(struct.monthFormatter.format(date)));
>> current.append('/');
>> current.append(struct.yearFormatter.format(date));
>> current.append(':');
>> current.append(struct.timeFormatter.format(date));
>> current.append(' ');
>> current.append(getTimeZone(date));
>> current.append(']');
>> struct.currentDateString = current.toString();
>> 
>> Instead of this one:
>> 
>> private SimpleDateFormat timeFormatter = new
>> SimpleDateFormat("[dd/MMM/yyyy:HH:mm:ss]");
>> ...
>> struct.currentDateString = struct.timeFormatter.format(date);
>> 
>> I¹ve added one more test with this structure to
>> org.apache.catalina.valves.Benchmarks and looks like the last solution the
>> shortest and fastest option. Plus we can pass any pattern from configuration
>> (#49165 enhancement intention):
>> 
>> TimeDateElementBenchmarkTest_Sync: 5 threads and 10000000 iterations using
>> Syncs took 3392ms
>> TimeDateElementBenchmarkTest_Local: 5 threads and 10000000 iterations using
>> ThreadLocals took 2551ms
>> TimeDateElementBenchmarkTest_LocalStruct: 5 threads and 10000000 iterations
>> using single ThreadLocal took 2237ms
>> TimeDateElementBenchmarkTest_LocalStruct_SBuilder: 5 threads and 10000000
>> iterations using single ThreadLocal, with StringBuilder took 2196ms
>> TimeDateElementBenchmarkTest_LocalStruct_SimpleDateFormat: 5 threads and
>> 10000000 iterations using single ThreadLocal SimpleDateFormat took 2152ms
>>  
>> 
>> P. S. My thanks to Mark Thomas for buglist to start.
>> 
>> 
>> 
> 
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
> 



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Bug 49165 - Allow %{TIME_FORMAT}t As Configuration for AccessLogValve

Posted by Mark Thomas <ma...@apache.org>.
On 15/07/2010 05:11, Alexander Shirkov wrote:
> Hello,
> 
> I¹m working on Bug 49164 (
> https://issues.apache.org/bugzilla/show_bug.cgi?id=49165)
> 
> Have a question: 
> Any hidden reasons, why following construction was used for creating %t
> pattern?:

It stems from this:
http://svn.apache.org/viewvc?view=revision&revision=285381

It appears to be performance related but from the figures you present
below, it may now be unnecessary.

Mark

> 
> private SimpleDateFormat dayFormatter = new SimpleDateFormat("dd");
> private SimpleDateFormat monthFormatter = new SimpleDateFormat("MM");
> private SimpleDateFormat yearFormatter = new SimpleDateFormat("yyyy");
> private SimpleDateFormat timeFormatter = new SimpleDateFormat("HH:mm:ss");
> ...
> StringBuilder current = new StringBuilder(32);
> current.append('[');
> current.append(struct.dayFormatter.format(date));
> current.append('/');
> current.append(lookup(struct.monthFormatter.format(date)));
> current.append('/');
> current.append(struct.yearFormatter.format(date));
> current.append(':');
> current.append(struct.timeFormatter.format(date));
> current.append(' ');
> current.append(getTimeZone(date));
> current.append(']');
> struct.currentDateString = current.toString();
> 
> Instead of this one:
> 
> private SimpleDateFormat timeFormatter = new
> SimpleDateFormat("[dd/MMM/yyyy:HH:mm:ss]");
> ...
> struct.currentDateString = struct.timeFormatter.format(date);
> 
> I¹ve added one more test with this structure to
> org.apache.catalina.valves.Benchmarks and looks like the last solution the
> shortest and fastest option. Plus we can pass any pattern from configuration
> (#49165 enhancement intention):
> 
> TimeDateElementBenchmarkTest_Sync: 5 threads and 10000000 iterations using
> Syncs took 3392ms
> TimeDateElementBenchmarkTest_Local: 5 threads and 10000000 iterations using
> ThreadLocals took 2551ms
> TimeDateElementBenchmarkTest_LocalStruct: 5 threads and 10000000 iterations
> using single ThreadLocal took 2237ms
> TimeDateElementBenchmarkTest_LocalStruct_SBuilder: 5 threads and 10000000
> iterations using single ThreadLocal, with StringBuilder took 2196ms
> TimeDateElementBenchmarkTest_LocalStruct_SimpleDateFormat: 5 threads and
> 10000000 iterations using single ThreadLocal SimpleDateFormat took 2152ms
>  
> 
> P. S. My thanks to Mark Thomas for buglist to start.
> 
> 
> 




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org