Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector Request.java

[Remy Maucherat]

Jan Luehe wrote:
Remy Maucherat wrote:
Bill Barker wrote:
----- Original Message ----- From: <[EMAIL PROTECTED]>
To: <[EMAIL PROTECTED]>
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?
Yes, it's sort of obvious ;)
How much memory does a formatter uses ? I'd like to point out that 
pasing of dates will happen relatively often (last modified headers in 
requests, etc).

To summarize:
- passing null is correct, but syncs (otherwise it wouldn't work)
- passing a thread local formatter is correct
- passing a shared formatter isn't correct (as there would be thread 
safety issues)

Rémy
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]