Solution found [Re: Performance with blocks protocol]

[Alexander Klimetschek <al...@mindquarry.com>]

Alexander Klimetschek schrieb:
>> Looking in the RequestProcessor that is called by the standard 
>> SitemapServlet, it calls ProcessingUtil.cleanup(), while leaving the 
>> processing of a request. I don't call that in the corresponding code 
>> in the o.a.c.sitemap.SitemapServlet, that might be a problem.
> 
> I have copied that, because I tried to figure out what is different 
> between the sitemap.SitemapServlet and the RequestProcessor, but it did 
> not change anything with the memory leak.

My fault, didn't built the core module after changing that. It is indeed 
the missing ProcessingUtil.cleanup() method. You probably did not put it 
into the sitemap.SitemapServlet because it cleans everything, including 
the current request data, so when called in a block that is called by 
another block, upon return no further processing is possible because you 
get NPEs when accessing the original HttpRequest...

So I put that call into the DispatcherServlet, right at the end of the 
service() method and it seems to work. We will now do some testing with 
the blocks-fw-samples and then commit it.


Alex

-- 
Alexander Klimetschek
http://www.mindquarry.com

Re: Solution found [Re: Performance with blocks protocol]

[Lars Trieloff <la...@trieloff.net>]

Alex,

please submit a patch to JIRA, I will review and test it with the  
same setting as yesterday evening (JMeter+Yourkit) post the figures  
and commit it if Daniel has no objections.

Lars

Am 08.11.2006 um 11:28 schrieb Alexander Klimetschek:

> Alexander Klimetschek schrieb:
>>> Looking in the RequestProcessor that is called by the standard  
>>> SitemapServlet, it calls ProcessingUtil.cleanup(), while leaving  
>>> the processing of a request. I don't call that in the  
>>> corresponding code in the o.a.c.sitemap.SitemapServlet, that  
>>> might be a problem.
>> I have copied that, because I tried to figure out what is  
>> different between the sitemap.SitemapServlet and the  
>> RequestProcessor, but it did not change anything with the memory  
>> leak.
>
> My fault, didn't built the core module after changing that. It is  
> indeed the missing ProcessingUtil.cleanup() method. You probably  
> did not put it into the sitemap.SitemapServlet because it cleans  
> everything, including the current request data, so when called in a  
> block that is called by another block, upon return no further  
> processing is possible because you get NPEs when accessing the  
> original HttpRequest...
>
> So I put that call into the DispatcherServlet, right at the end of  
> the service() method and it seems to work. We will now do some  
> testing with the blocks-fw-samples and then commit it.
>
>
> Alex
>
> -- 
> Alexander Klimetschek
> http://www.mindquarry.com
>

--
Lars Trieloff
visit http://www.mindquarry.com/

Re: Solution found [Re: Performance with blocks protocol]

[Daniel Fagerstrom <da...@nada.kth.se>]

Alexander Klimetschek skrev:
> Alexander Klimetschek schrieb:
>>> Looking in the RequestProcessor that is called by the standard 
>>> SitemapServlet, it calls ProcessingUtil.cleanup(), while leaving the 
>>> processing of a request. I don't call that in the corresponding code 
>>> in the o.a.c.sitemap.SitemapServlet, that might be a problem.
>>
>> I have copied that, because I tried to figure out what is different 
>> between the sitemap.SitemapServlet and the RequestProcessor, but it 
>> did not change anything with the memory leak.
> 
> My fault, didn't built the core module after changing that. It is indeed 
> the missing ProcessingUtil.cleanup() method.

Great!

> You probably did not put it 
> into the sitemap.SitemapServlet because it cleans everything, including 
> the current request data, so when called in a block that is called by 
> another block, upon return no further processing is possible because you 
> get NPEs when accessing the original HttpRequest...

That was a kind interpretation :) I didn't understand the importance of 
the ProcessingUtil.cleanup() and probably just forgot to call it.

AFAICS the ProcessingUtil.cleanup() is only used for handling poolable 
beans in the end of the request.

> So I put that call into the DispatcherServlet, right at the end of the 
> service() method and it seems to work. We will now do some testing with 
> the blocks-fw-samples and then commit it.

We should merge the servlet.SitemapServlet and the 
sitemap.SitemapServlet and then this solution doesn't work. But right 
now the important thing is to get rid of the memory leak so we can solve 
it like that for the moment.

In the long term the handling poolable components probably need some 
oversight. IIRC there have been discussions on the list that most of the 
sitemap components should be created by a factory instead of being 
poolable (see [Generator|Transformer|Serializer]Factory).

Also Spring doesn't have the concept of returning a component to the 
container as Avalon has (ServiceManager.release(Object o)). So it is not 
possible to implement true poolables. Instead the poolables are 
destroyed in the end of the request. Which makes them closer to request 
scoped components in Spring.

One possibility would be to put the ProcessingUtil.cleanup() in a 
servlet request listners destroy method.

/Daniel