You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by Ralph Goers <Ra...@dslextreme.com> on 2005/05/13 01:50:35 UTC
Re: svn commit: r169856 - /cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/environment/http/HttpRequest.java
Why is "sessions" a synchronized map if you are only accessing it in a
block synchronized on the session. I would much prefer that you
a) not use a synchronized map
b) synchronize on the map instead of the session.
Is there a reason that this wouldn't work?
Ralph
joerg@apache.org wrote:
>Author: joerg
>Date: Thu May 12 10:50:20 2005
>New Revision: 169856
>
>URL: http://svn.apache.org/viewcvs?rev=169856&view=rev
>Log:
>fixed weak referencing (thanks to Alfred Nathaniel)
>
>Modified:
> cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/environment/http/HttpRequest.java
>
>Modified: cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/environment/http/HttpRequest.java
>URL: http://svn.apache.org/viewcvs/cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/environment/http/HttpRequest.java?rev=169856&r1=169855&r2=169856&view=diff
>==============================================================================
>--- cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/environment/http/HttpRequest.java (original)
>+++ cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/environment/http/HttpRequest.java Thu May 12 10:50:20 2005
>@@ -230,11 +230,11 @@
> // synch on server session assures only one wrapper per session
> synchronized (serverSession) {
> // retrieve existing wrapper
>- session = (HttpSession)sessions.get(serverSession);
>+ session = (HttpSession)((WeakReference)sessions.get(serverSession)).get();
> if (session == null) {
> // create new wrapper
> session = new HttpSession(serverSession);
>- sessions.put(serverSession, session);
>+ sessions.put(serverSession, new WeakReference(session));
> }
> }
> } else {
>
>
>
>
Re: svn commit: r169856 - /cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/environment/http/HttpRequest.java
Posted by Ralph Goers <Ra...@dslextreme.com>.
Joerg Heinicke wrote:
>
>It would work, but IMO the current implementation is better because it is more
>fine-grained. Synchronizing the block on the map (yes, you don't need a
>synchronized map then) blocks all requests for the execution of this block while
>this impl blocks only the requests for the current server session. Only the
>access to the map itself must be blocked for all requests.
>
>Joerg
>
>
This is somewhat of an illusion. The only other thing happening is the
allocation of the HttpSession when required. The idea that synchronizing
around the whole operation is worse than synchronzing twice to
manipulate the map - and synchronizing the session, is going to make a
noticable performance difference just doesn't fly.
Re: svn commit: r169856 - /cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/environment/http/HttpRequest.java
Posted by Joerg Heinicke <jo...@gmx.de>.
Ralph Goers <Ralph.Goers <at> dslextreme.com> writes:
> Why is "sessions" a synchronized map if you are only accessing it in a
> block synchronized on the session. I would much prefer that you
> a) not use a synchronized map
> b) synchronize on the map instead of the session.
>
> Is there a reason that this wouldn't work?
It would work, but IMO the current implementation is better because it is more
fine-grained. Synchronizing the block on the map (yes, you don't need a
synchronized map then) blocks all requests for the execution of this block while
this impl blocks only the requests for the current server session. Only the
access to the map itself must be blocked for all requests.
Joerg