You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2010/11/16 19:07:56 UTC
svn commit: r1035720 -
/tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java
Author: markt
Date: Tue Nov 16 18:07:56 2010
New Revision: 1035720
URL: http://svn.apache.org/viewvc?rev=1035720&view=rev
Log:
Session manager performance
Move sync. No performance change but sync requirements for sub-classes are clearer.
Still looking to remove sync completely.
Modified:
tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java
Modified: tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java?rev=1035720&r1=1035719&r2=1035720&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java (original)
+++ tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java Tue Nov 16 18:07:56 2010
@@ -959,7 +959,7 @@ public abstract class ManagerBase extend
}
- protected void getRandomBytes(byte bytes[]) {
+ protected synchronized void getRandomBytes(byte bytes[]) {
// Generate a byte array containing a session identifier
if (devRandomSourceIsValid && randomIS == null) {
setRandomFile(devRandomSource);
@@ -1008,9 +1008,7 @@ public abstract class ManagerBase extend
}
while (resultLenBytes < this.sessionIdLength) {
- synchronized (this) {
- getRandomBytes(random);
- }
+ getRandomBytes(random);
MessageDigest md = digests.poll();
if (md == null) {
// If this fails, NPEs will follow. This should never fail
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1035720 - /tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java
Posted by Mark Thomas <ma...@apache.org>.
On 16/11/2010 18:35, sebb wrote:
> On 16 November 2010 18:07, <ma...@apache.org> wrote:
>> Author: markt
>> Date: Tue Nov 16 18:07:56 2010
>> New Revision: 1035720
>>
>> URL: http://svn.apache.org/viewvc?rev=1035720&view=rev
>> Log:
>> Session manager performance
>> Move sync. No performance change but sync requirements for sub-classes are clearer.
>> Still looking to remove sync completely.
> Does the method pre-amble (as above) need to be synch?
> AFAICT, most other accesses to devRandomSource and randomIS within the
> module are not protected by synch.
Maybe. There may be issues around changing devRandomSource whilst the
application is running. That isn't the focus of what I am looking at
right now (and I am not sure that should be supported anyway) but I am
keeping it in mind.
My intention is to reduce the scope of the sync as far as possible and
ideally remove it. I am doing this via small incremental changes rather
than one big commit so folks can follow what I am doing and how I am
doing it.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1035720 - /tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java
Posted by sebb <se...@gmail.com>.
On 16 November 2010 18:07, <ma...@apache.org> wrote:
> Author: markt
> Date: Tue Nov 16 18:07:56 2010
> New Revision: 1035720
>
> URL: http://svn.apache.org/viewvc?rev=1035720&view=rev
> Log:
> Session manager performance
> Move sync. No performance change but sync requirements for sub-classes are clearer.
> Still looking to remove sync completely.
>
> Modified:
> tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java
>
> Modified: tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java?rev=1035720&r1=1035719&r2=1035720&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java (original)
> +++ tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java Tue Nov 16 18:07:56 2010
> @@ -959,7 +959,7 @@ public abstract class ManagerBase extend
> }
>
>
> - protected void getRandomBytes(byte bytes[]) {
> + protected synchronized void getRandomBytes(byte bytes[]) {
> // Generate a byte array containing a session identifier
> if (devRandomSourceIsValid && randomIS == null) {
> setRandomFile(devRandomSource);
Does the method pre-amble (as above) need to be synch?
AFAICT, most other accesses to devRandomSource and randomIS within the
module are not protected by synch.
> @@ -1008,9 +1008,7 @@ public abstract class ManagerBase extend
> }
>
> while (resultLenBytes < this.sessionIdLength) {
> - synchronized (this) {
> - getRandomBytes(random);
> - }
> + getRandomBytes(random);
> MessageDigest md = digests.poll();
> if (md == null) {
> // If this fails, NPEs will follow. This should never fail
>
>
>
> ---------------------------------------------------------------------
> 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