tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Thomas <ma...@apache.org>
Subject Re: svn commit: r1036595 - in /tomcat/trunk: java/org/apache/catalina/ java/org/apache/catalina/ha/session/ java/org/apache/catalina/session/ test/org/apache/catalina/session/
Date Thu, 25 Nov 2010 16:33:19 GMT
On 25/11/2010 16:10, Remy Maucherat wrote:
> On Thu, 2010-11-18 at 19:59 +0000, markt@apache.org wrote:
>> Author: markt
>> Date: Thu Nov 18 19:59:11 2010
>> New Revision: 1036595
>>
>> URL: http://svn.apache.org/viewvc?rev=1036595&view=rev
>> Log:
>> Fix expiration statistics broken by r1036281
>> Add session creation and expiration rate statistics based on the 100 most recently
created/expired sessions
>> Modify average session alive time to also use 100 most recently expired sessions
>> Update benchmarks - new statistics add overhead but not significant in overall processing
chain
> 
> But going back to the original optimization work, I still don't quite
> understand. As Tim pointed out, the MD5 hash is probably bad, and

I wouldn't call it bad. It doesn't do any harm (apart from adding a very
small amount of overhead), and it would help if the random source
selected ended up not being that random.

I thought the trade-off of protection against bad choices of random
sources was worth the minimal overhead added. I'm not against removing
it entirely, if there is consensus to do so.

> SecureRandom is already internally thread safe. So the simplest
> refactoring (remove the hash and the synchroinized block) was not
> tested. Is it really a big enough bottleneck which would need the more
> complex plumbing to parallelize ?

For SecureRandom, yes. I did test this locally and it achieves
thread-safety by using an internal sync and it did create a significant
bottleneck. That is why I went the parallel route. Reading from a stream
has a similar sync so again that is why I went the parallel route.

How about this as an approach to reduce the complexity:
1. Remove the MD5 code (optional)
2. Default to /dev/urandom then SecureRandom. Don't fall back to Random.
3. Provide a class that implements Random that reads data from a file
4. If randomFile is specified, use the the class from 3 as the Random source

That should reduce the current 3 Queues to one. I doubt it will improve
performance much but it should make the code clearer.

Thoughts?

Mark

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


Mime
View raw message