Re: Javaflow - major memory issue
On 28.03.2008 04:55, Torsten Curdt wrote: The output I showed pointed to org.apache.cocoon.components.flow.java.Continuation which only seems to exist in Cocoon 2.1. Nothing unsets the context there. Hah - well spotted! Having a look into the code continuations are only handled by JavaInterpreter. There are two methods callFunction(..) and handleContinuation(..) calling Continuation.registerThread() and deregisterThread() in a finally block. From a brief look I have no idea if I can just unset the ContinuationContext there as well. You might know more about it. We should add a try/finally block in Continuation.suspend() that clears the context after a suspend. That should fix it. Unfortunately, it is not possible to unset the ContinuationContext completely. It's needed in JavaInterpreter.handleContinuation(..) when a child continuation is created: Continuation parentContinuation = (Continuation) parentwk.getContinuation(); ContinuationContext parentContext = (ContinuationContext) parentContinuation.getContext(); ContinuationContext context = new ContinuationContext(); context.setObject(parentContext.getObject()); context.setMethod(parentContext.getMethod()); Without completely rewriting it the only thing I did was to remove the data in the ContinuationContext that is not necessary. I do this by an extra call to ContinuationContext.onSuspend() in AbstractContinuable since Continuation is not aware of the implementation of its context (it's just an Object). Please review my changes [1] because I'm not really sure about them. They work for the normal case, but what happens in an error case? I can't see what's really going on except that the method is left on Continuation.suspend() ... It was very interesting to debug it when AbstractContinuable.sendPageAndWait(..) was actually hit twice. I guess this handling is different in 2.2. There a clean ContinuationContext is created on both callFunction(..) and handleContinuation(..). Joerg [1] http://svn.apache.org/viewvc?rev=642694view=rev
Re: Javaflow - major memory issue
On Mar 30, 2008, at 09:43, Joerg Heinicke wrote: On 28.03.2008 04:55, Torsten Curdt wrote: The output I showed pointed to org.apache.cocoon.components.flow.java.Continuation which only seems to exist in Cocoon 2.1. Nothing unsets the context there. Hah - well spotted! Having a look into the code continuations are only handled by JavaInterpreter. There are two methods callFunction(..) and handleContinuation(..) calling Continuation.registerThread() and deregisterThread() in a finally block. From a brief look I have no idea if I can just unset the ContinuationContext there as well. You might know more about it. We should add a try/finally block in Continuation.suspend() that clears the context after a suspend. That should fix it. Unfortunately, it is not possible to unset the ContinuationContext completely. It's needed in JavaInterpreter.handleContinuation(..) when a child continuation is created: Continuation parentContinuation = (Continuation) parentwk.getContinuation(); ContinuationContext parentContext = (ContinuationContext) parentContinuation.getContext(); ContinuationContext context = new ContinuationContext(); context.setObject(parentContext.getObject()); context.setMethod(parentContext.getMethod()); There is no need to really obtain that value from the parent. If handleContinuation would have also the function parameter it could use the same initialization as in callFuntion. Actually a way of fixing this would be to add two Strings to the Continuation class. One holding the classname, the other holding the method name. And then do context.setObject(userScopes.get(parentContinuation.getScopeName())); context.setMethod(methods.get(parentContinuation.getFunctionName())); in handleContinuation. Then the cleaning of the context should be fine. Without completely rewriting it the only thing I did was to remove the data in the ContinuationContext that is not necessary. I do this by an extra call to ContinuationContext.onSuspend() in AbstractContinuable since Continuation is not aware of the implementation of its context (it's just an Object). Please review my changes [1] because I'm not really sure about them. Not a fan of the Collections.synchronizedMap(new HashMap()) change - but other than that they look OK on the first glance :) They work for the normal case, but what happens in an error case? I can't see what's really going on except that the method is left on Continuation.suspend() ... It was very interesting to debug it when AbstractContinuable.sendPageAndWait(..) was actually hit twice. :) ...what error case do you mean? I guess this handling is different in 2.2. There a clean ContinuationContext is created on both callFunction(..) and handleContinuation(..). Indeed ...that would be another fix ...porting it from 2.2 :) Thanks for looking into that. cheers -- Torsten
Re: Javaflow - major memory issue
On 30.03.2008 07:35, Torsten Curdt wrote: There is no need to really obtain that value from the parent. If handleContinuation would have also the function parameter it could use the same initialization as in callFuntion. Yes, if the function name would have been available ... This was beyond what I wanted to do. But since you confirmed it now I risked this change. Actually a way of fixing this would be to add two Strings to the Continuation class. One holding the classname, the other holding the method name. And then do context.setObject(userScopes.get(parentContinuation.getScopeName())); context.setMethod(methods.get(parentContinuation.getFunctionName())); The scopes are by Class. Therefore I only stored the function name and retrieved the method. in handleContinuation. Then the cleaning of the context should be fine. Yes, it's better now. Without completely rewriting it the only thing I did was to remove the data in the ContinuationContext that is not necessary. I do this by an extra call to ContinuationContext.onSuspend() in AbstractContinuable since Continuation is not aware of the implementation of its context (it's just an Object). Please review my changes [1] because I'm not really sure about them. Not a fan of the Collections.synchronizedMap(new HashMap()) change - but Just curious, any reason for this? Is it not as optimized? other than that they look OK on the first glance :) They work for the normal case, but what happens in an error case? I can't see what's really going on except that the method is left on Continuation.suspend() ... It was very interesting to debug it when AbstractContinuable.sendPageAndWait(..) was actually hit twice. :) ...what error case do you mean? Not sure, originally you proposed to put it into a try finally block. I guess this handling is different in 2.2. There a clean ContinuationContext is created on both callFunction(..) and handleContinuation(..). Indeed ...that would be another fix ...porting it from 2.2 :) That's really beyond what I want to do ;-) Let's leave the good stuff for 2.2. Thanks for looking into that. It was really interesting to see how a Java method is cut into two pieces. Suddenly the debugger stops stepping, but just leaves the methods. And on the next call it just jumps to the appropriate places in the code. Once you get used to it it's easy to debug. I have never tried it before but now I like it better than flowscript. Joerg
Re: Javaflow - major memory issue
Without completely rewriting it the only thing I did was to remove the data in the ContinuationContext that is not necessary. I do this by an extra call to ContinuationContext.onSuspend() in AbstractContinuable since Continuation is not aware of the implementation of its context (it's just an Object). Please review my changes [1] because I'm not really sure about them. Not a fan of the Collections.synchronizedMap(new HashMap()) change - but Just curious, any reason for this? Is it not as optimized? Well, of course you pay a runtime penalty for another method invocation unless the hotspots is able to optimize that away. Not sure. But it's less about optimization - more about keeping the synchronization more visible. But as long as the access to the hashmap is simple like it is right now I would not argue about it :) other than that they look OK on the first glance :) They work for the normal case, but what happens in an error case? I can't see what's really going on except that the method is left on Continuation.suspend() ... It was very interesting to debug it when AbstractContinuable.sendPageAndWait(..) was actually hit twice. :) ...what error case do you mean? Not sure, originally you proposed to put it into a try finally block. Just to have it as post condition. You never know. I guess this handling is different in 2.2. There a clean ContinuationContext is created on both callFunction(..) and handleContinuation(..). Indeed ...that would be another fix ...porting it from 2.2 :) That's really beyond what I want to do ;-) Let's leave the good stuff for 2.2. +1 Thanks for looking into that. It was really interesting to see how a Java method is cut into two pieces. Suddenly the debugger stops stepping, but just leaves the methods. And on the next call it just jumps to the appropriate places in the code. Once you get used to it it's easy to debug. I have never tried it before but now I like it better than flowscript. Yeah, it's interesting isn't it? :) cheers -- Torsten
Re: Javaflow - major memory issue
On Mar 28, 2008, at 04:29, Joerg Heinicke wrote: On 27.03.2008 10:33, Torsten Curdt wrote: Just have a look at the quote from the original. It gives the object relationships in the memory. BufferedOutputStream takes 50% and is hold in HttpEnvironment which is hold in TreeProcessorRedirector which is hold in ContinuationContext. I wondered why it is there. Ah ...well, the ContinuationContext should be short living. Maybe there is a dangling reference to it? Ah, getting closer :) It's the Continuation that holds the ContinuationContext [1]: Hm... it should set clear the reference in 'finally'. See the execute method http://svn.apache.org/viewvc/commons/sandbox/javaflow/trunk/src/java/org/apache/commons/javaflow/bytecode/StackRecorder.java?revision=560660view=markup The output I showed pointed to org.apache.cocoon.components.flow.java.Continuation which only seems to exist in Cocoon 2.1. Nothing unsets the context there. Hah - well spotted! Having a look into the code continuations are only handled by JavaInterpreter. There are two methods callFunction(..) and handleContinuation(..) calling Continuation.registerThread() and deregisterThread() in a finally block. From a brief look I have no idea if I can just unset the ContinuationContext there as well. You might know more about it. We should add a try/finally block in Continuation.suspend() that clears the context after a suspend. That should fix it. cheers -- Torsten
Re: Javaflow - major memory issue
On Mar 27, 2008, at 05:31, Joerg Heinicke wrote: On 18.03.2008 03:07, footh wrote: Sure, here is the hierarchy from bottom to top. At this point, I ran the test for about five minutes (running longer would increase the percentage) and the retained size of the one ContinuationsManagerImpl object is 58% of the total. The BufferedOutputStream is 50% of the total, so the other 8% is consumed by the objects in between. org.apache.cocoon.util.BufferedOutputStream secureOutputStream of org.apache.cocoon.environment.http.HttpEnvironment env of org.apache.cocoon.components.treeprocessor.ConcreteTreeProcessor $TreeProcessorRedirector redirector of org.apache.cocoon.components.flow.java.ContinuationContext What I was so much concerned about here was the fact of storing the whole environment in the continuation, especially since we have this non-flushing BufferedOutputStream at the end. Is there any point in storing the environment? Do we get anything useful out of it after continueing the continuation? What do you mean by environment ...it's not like the whole jvm is stored but only the flow. External resources should be injected (vs stored) as much as possible. cheers -- Torsten
Re: Javaflow - major memory issue
On 27.03.2008 04:39, Torsten Curdt wrote: Sure, here is the hierarchy from bottom to top. At this point, I ran the test for about five minutes (running longer would increase the percentage) and the retained size of the one ContinuationsManagerImpl object is 58% of the total. The BufferedOutputStream is 50% of the total, so the other 8% is consumed by the objects in between. org.apache.cocoon.util.BufferedOutputStream secureOutputStream of org.apache.cocoon.environment.http.HttpEnvironment env of org.apache.cocoon.components.treeprocessor.ConcreteTreeProcessor$TreeProcessorRedirector redirector of org.apache.cocoon.components.flow.java.ContinuationContext What I was so much concerned about here was the fact of storing the whole environment in the continuation, especially since we have this non-flushing BufferedOutputStream at the end. Is there any point in storing the environment? Do we get anything useful out of it after continueing the continuation? What do you mean by environment ...it's not like the whole jvm is stored but only the flow. External resources should be injected (vs stored) as much as possible. Just have a look at the quote from the original. It gives the object relationships in the memory. BufferedOutputStream takes 50% and is hold in HttpEnvironment which is hold in TreeProcessorRedirector which is hold in ContinuationContext. I wondered why it is there. Joerg
Re: Javaflow - major memory issue
On Mar 27, 2008, at 13:18, Joerg Heinicke wrote: On 27.03.2008 04:39, Torsten Curdt wrote: Sure, here is the hierarchy from bottom to top. At this point, I ran the test for about five minutes (running longer would increase the percentage) and the retained size of the one ContinuationsManagerImpl object is 58% of the total. The BufferedOutputStream is 50% of the total, so the other 8% is consumed by the objects in between. org.apache.cocoon.util.BufferedOutputStream secureOutputStream of org.apache.cocoon.environment.http.HttpEnvironment env of org.apache.cocoon.components.treeprocessor.ConcreteTreeProcessor $TreeProcessorRedirector redirector of org.apache.cocoon.components.flow.java.ContinuationContext What I was so much concerned about here was the fact of storing the whole environment in the continuation, especially since we have this non-flushing BufferedOutputStream at the end. Is there any point in storing the environment? Do we get anything useful out of it after continueing the continuation? What do you mean by environment ...it's not like the whole jvm is stored but only the flow. External resources should be injected (vs stored) as much as possible. Just have a look at the quote from the original. It gives the object relationships in the memory. BufferedOutputStream takes 50% and is hold in HttpEnvironment which is hold in TreeProcessorRedirector which is hold in ContinuationContext. I wondered why it is there. Ah ...well, the ContinuationContext should be short living. Maybe there is a dangling reference to it? (Let's stop cross posting and move over to the dev list) cheers -- Torsten
Re: Javaflow - major memory issue
Torsten Curdt tcurdt at apache.org writes: Just have a look at the quote from the original. It gives the object relationships in the memory. BufferedOutputStream takes 50% and is hold in HttpEnvironment which is hold in TreeProcessorRedirector which is hold in ContinuationContext. I wondered why it is there. Ah ...well, the ContinuationContext should be short living. Maybe there is a dangling reference to it? Ah, getting closer :) It's the Continuation that holds the ContinuationContext [1]: org.apache.cocoon.util.BufferedOutputStream secureOutputStream of org.apache.cocoon.environment.http.HttpEnvironment env of org.apache.cocoon.components.treeprocessor.ConcreteTreeProcessor $TreeProcessorRedirector redirector of org.apache.cocoon.components.flow.java.ContinuationContext context of org.apache.cocoon.components.flow.java.Continuation continuation of org.apache.cocoon.components.flow.WebContinuation Joerg [1] http://marc.info/?l=xml-cocoon-usersm=120582409910104w=4
Re: Javaflow - major memory issue
On Mar 27, 2008, at 15:13, Joerg Heinicke wrote: Torsten Curdt tcurdt at apache.org writes: Just have a look at the quote from the original. It gives the object relationships in the memory. BufferedOutputStream takes 50% and is hold in HttpEnvironment which is hold in TreeProcessorRedirector which is hold in ContinuationContext. I wondered why it is there. Ah ...well, the ContinuationContext should be short living. Maybe there is a dangling reference to it? Ah, getting closer :) It's the Continuation that holds the ContinuationContext [1]: Hm... it should set clear the reference in 'finally'. See the execute method http://svn.apache.org/viewvc/commons/sandbox/javaflow/trunk/src/java/org/apache/commons/javaflow/bytecode/StackRecorder.java?revision=560660view=markup Weird...
Re: Javaflow - major memory issue
On Mar 27, 2008, at 15:33, Torsten Curdt wrote: On Mar 27, 2008, at 15:13, Joerg Heinicke wrote: Torsten Curdt tcurdt at apache.org writes: Just have a look at the quote from the original. It gives the object relationships in the memory. BufferedOutputStream takes 50% and is hold in HttpEnvironment which is hold in TreeProcessorRedirector which is hold in ContinuationContext. I wondered why it is there. Ah ...well, the ContinuationContext should be short living. Maybe there is a dangling reference to it? Ah, getting closer :) It's the Continuation that holds the ContinuationContext [1]: Hm... it should set clear the reference in 'finally'. See the execute method http://svn.apache.org/viewvc/commons/sandbox/javaflow/trunk/src/java/org/apache/commons/javaflow/bytecode/StackRecorder.java?revision=560660view=markup Weird... What cocoon version are we talking about anyway?
Re: Javaflow - major memory issue
On 27.03.2008 10:33, Torsten Curdt wrote: Just have a look at the quote from the original. It gives the object relationships in the memory. BufferedOutputStream takes 50% and is hold in HttpEnvironment which is hold in TreeProcessorRedirector which is hold in ContinuationContext. I wondered why it is there. Ah ...well, the ContinuationContext should be short living. Maybe there is a dangling reference to it? Ah, getting closer :) It's the Continuation that holds the ContinuationContext [1]: Hm... it should set clear the reference in 'finally'. See the execute method http://svn.apache.org/viewvc/commons/sandbox/javaflow/trunk/src/java/org/apache/commons/javaflow/bytecode/StackRecorder.java?revision=560660view=markup The output I showed pointed to org.apache.cocoon.components.flow.java.Continuation which only seems to exist in Cocoon 2.1. Nothing unsets the context there. Having a look into the code continuations are only handled by JavaInterpreter. There are two methods callFunction(..) and handleContinuation(..) calling Continuation.registerThread() and deregisterThread() in a finally block. From a brief look I have no idea if I can just unset the ContinuationContext there as well. You might know more about it. Joerg
Re: Javaflow - major memory issue
On 18.03.2008 03:07, footh wrote: Sure, here is the hierarchy from bottom to top. At this point, I ran the test for about five minutes (running longer would increase the percentage) and the retained size of the one ContinuationsManagerImpl object is 58% of the total. The BufferedOutputStream is 50% of the total, so the other 8% is consumed by the objects in between. org.apache.cocoon.util.BufferedOutputStream secureOutputStream of org.apache.cocoon.environment.http.HttpEnvironment env of org.apache.cocoon.components.treeprocessor.ConcreteTreeProcessor$TreeProcessorRedirector redirector of org.apache.cocoon.components.flow.java.ContinuationContext What I was so much concerned about here was the fact of storing the whole environment in the continuation, especially since we have this non-flushing BufferedOutputStream at the end. Is there any point in storing the environment? Do we get anything useful out of it after continueing the continuation? Joerg
