tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jan Luehe <Jan.Lu...@Sun.COM>
Subject Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector Request.java
Date Mon, 09 Aug 2004 17:24:29 GMT
Remy Maucherat wrote:
> Bill Barker wrote:
> 
>> ----- Original Message ----- From: <luehe@apache.org>
>> To: <jakarta-tomcat-catalina-cvs@apache.org>
>> Sent: Thursday, August 05, 2004 6:27 PM
>> Subject: cvs commit:
>> jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector
>> Request.java
>>
>>
>>  
>>
>>> luehe       2004/08/05 18:27:50
>>>
>>>  Modified:    catalina/src/share/org/apache/catalina/connector
>>>                        Request.java
>>>  Log:
>>>  Avoid allocating SimpleDateFormat[] for each request. Instead, declare
>>>   
>>
>> SimpleDateFormat[] as static and use static initializer to initialize it.
>>  
>>
>>
>> -1.  SimpleDateFormat isn't thread-safe, so you can't have multiple 
>> threads
>> accessing them.  That's why they were instance variables, so that one one
>> thread would ever be accessing them.

OK.

>>>  This is consistent with SimpleDateFormat[] in
>>>   
>>
>> org.apache.tomcat.util.http.FastHttpDateFormat.
>>
>> Which is well known to be broken ;-).
>>  
>>
> There's a misunderstanding about the design (I don't think 
> FastHttpDateFormat is that broken anyway ;) ). The formats passed in 
> argument has to be thread local, and FHDF won't sync.
> 
> So -1 too.


actually, FHDF.parseDate() does sync around SimpleDateFormat.parse()
if the "threadLocalformats" passed to it is null, in which case it
falls back to its own static "formats" (notice that in this case,
FHDF.internalParseDate() is within the synchronized block):

     public static final long parseDate(String value,
                                        DateFormat[] threadLocalformats) {

         ....

         if (threadLocalformats != null) {
             date = internalParseDate(value, threadLocalformats);
             synchronized (parseCache) {
                 updateCache(parseCache, value, date);
             }
         } else {
             synchronized (parseCache) {
                 date = internalParseDate(value, formats);
                 updateCache(parseCache, value, date);
             }
         }
     }

The DateFormat[] passed in from Request.parseDate() is *identical* to
the static "formats" in FHDF, so why are we not just passing "null"
from Request.parseDate() and leverage FHDF.formats (instead of having
each Request object provide its own SimpleDateFormat[] instance var)?

Is it to avoid that the potentially expensive date parsing is
performed inside the synchronized block?

Thanks,


Jan


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


Mime
View raw message