History | Log In     View a printable version of the current page.  
Issue Details (XML | Word | Printable)

Key: CACHE-214
Type: Improvement Improvement
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Lars Torunski
Reporter: Konstantin Polyzois
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
OSCache

Reduce memory consumption of ResponseContent

Created: 05/Nov/05 05:47 AM   Updated: 12/Feb/06 06:49 AM
Component/s: Base Classes
Affects Version/s: 2.1.1
Fix Version/s: 2.3

Environment: Java

Flags: Important


 Description  « Hide
Hi! We have been doing some memory-profiling of our site. And found a issue with the class com.opensymphony.oscache.web.filter.RepsonseContent

<a href="https://xwork.dev.java.net/source/browse/oscache/src/java/com/opensymphony/oscache/web/filter/ResponseContent.java?rev=1.1&view=auto&content-type=text/vnd.viewcvs-markup" >com.opensymphony.oscache.web.filter.RepsonseContent</a>.

The issue involves the field named 'bout' declared thus:

private transient ByteArrayOutputStream bout = new ByteArrayOutputStream(1000);
    private Locale locale = null;
    private String contentEncoding = null;
    private String contentType = null;
    private byte[] content = null;
    private long lastModified = -1;

'bout' will after a call hold the cached content. It will also be in the field 'content'. 'bout' is transient but that will not help until we do a restart of the jvm.

I propose that on commit the filed 'bout' be cleared something like this:

/**
     * Called once the response has been written in its entirety. This
     * method commits the response output stream by converting the output
     * stream into a byte array.
     */
    public void commit() {
        content = bout.toByteArray();
        bout=null; //<---- New code
    }

This will in affect half the memory consumption of cached objects!
By the way we used the Yourkit Java profiler it is a great product!

/Konstantin

PS If you want to I can submit a patch or try out the change and make sure it works!




 All   Comments   Change History      Sort Order:
Lars Torunski - [06/Nov/05 01:46 AM ]
Your patch can be part of a 2.2.1 release, but we have to avoid that commit() is invoked more than once and have to check the effects when getOutputStream() is invoked when bout is null.

Konstantin Polyzois - [06/Nov/05 02:07 PM ]
Do you wish me to do it? With some defensive coding as outlined by you?

Lars Torunski - [07/Nov/05 10:08 PM ]
Yes, please consider ResponseContent.getOutputStream() can return NULL now. Maybe CacheHttpServletResponseWrapper should implement the isCommited() method based on when getContent() was invoked before and hence the ResponseContent was comitted.

Konstantin Polyzois - [17/Nov/05 03:15 PM ]
I am at loss I want to add a test to the unittests but the test-web demands a servletengine e.g tomcat but my test is just a simple assert. Where should a simple unit test be added?

Lars Torunski - [18/Nov/05 12:27 AM ]
Tests belonging to the CacheFilter should be included in the test-web always.
The fix is included in the current CVS HEAD.