-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Jason,

On 3/2/2010 7:21 PM, Jason Brittain wrote:
>> Why does the request have to be an HTTP request in order to have the
>> access log run?
> 
> That does seem to be a bug.

Note that this is not actually a part of the AccessLogValve, it's just
part of Xie's implementation.

> By default, the access logger logs the common
> log web server
> format, but of course it doesn't have to, so it should log non-HTTP requests
> as well, but maybe
> only if a non-default pattern is configured?

Fair enough: most of the information you'd want to log is from HTTP
requests (like the URI, for instance). The only things that are
available for non-HTTP requests are:

- - current date/time
- - transaction time
- - number of bytes read and sent
- - local address
- - remote address
- - request attributes
- - server name

Actually, that's quite a bit, but I've never seen an HTTP log that
doesn't log the URI :)

>>> long t2 = System.currentTimeMillis();
>>> long time = t2 - t1;
>>
>> This isn't your choice, it's in the original code, but why not just do:
>>
>> long elapsed = System.currentTimeMillis();
>> ...
>> elapsed = System.currentTimeMillis() - elapsed;
>>
>> ??
>>
>> Fewer items on the stack, etc.
>>
> 
> Except that then it is more difficult to debug.  Right?  It isn't as easy to
> inspect the value of
> the current time if you perform the subtraction without first assigning the
> current time to a
> variable.

Fair enough, though debugging this timing code shouldn't really be required.

>>  >     private Date getDate() {
>>>         // Only create a new Date once per second, max.
>>>         long systime = System.currentTimeMillis();
>>>         AccessDateStruct struct = currentDateStruct.get();
>>>         if ((systime - struct.currentDate.getTime()) > 1000) {
>>>             struct.currentDate.setTime(systime);
>>>             struct.currentDateString = null;
>>>         }
>>>         return struct.currentDate;
>>>     }
>>
>> I don't understand why this is ThreadLocal, instead of just synchronized
>> across the object. Maybe it's slightly faster to avoid the
>> synchronization and just use ThreadLocals, but I'm not sure how many
>> requests per second a single Thread is going to process, so I'm not
>> convinced that caching this data is worth the complexity it requires in
>> this class. I'd love to hear from a Tomcat dev about this.
>>
> 
> Tomcat can (hopefully) answer a larger number of requests per second
> every year on decently modern hardware.  Benchmark it both ways on
> a reasonably good/wide machine and you'll see why avoiding the sync
> is helpful.  I don't think it muddies the code very badly here.

Okay. Certainly avoiding object creation is a good idea, and avoiding
highly-contended synchronization is a good idea, too. I'd like to see a
performance comparison between these strategies, though. Maybe I'll run
one :)

> The %b portion of the Combined Log Format is documented to be the "size of
> the object returned to the client, not including the response headers."
> http://httpd.apache.org/docs/1.3/logs.html#common

Right. Presumably, the Content-Length is synonymous with the above, but
it might not be. Also, Content-Length is not always set, so you'll get a
lot of "-" written in the log even when response bodies actually has
content. In this implementation, "%b" is equivalent to "%{Content-Length}o".

Counting bytes isn't that big of a deal, either. I'll submit a patch at
some point... maybe using a different pattern character.

- -chris
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkuOjCAACgkQ9CaO5/Lv0PAAmgCgt9QVocFjXVNB3t4ib6fWOUIL
++YAoKdpBuT1uhobAIxasRsdw/PaK00e
=zS1q
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org

Reply via email to