On Nov 1, 2013, at 6:23 AM, Scott Severtson <ssevert...@digitalmeasures.com> wrote:
> All, > > We've noticed that our Tomcat 6.0.37 instances have increasing overhead the > longer they stay up. We've captured some heap dumps, expecting to find memory > leaks within our code, but found something different instead. > > Jasper's BodyContentImpl is actually holding most of the non-collectible > garbage, in the internal char[] buffer. I've checked trunk of Tomcat 6, 7 and > 8, and see that the same implementation is still used, so this issue applies > to all currently supported versions. > > This issue appears to be well known: > * > http://stackoverflow.com/questions/10421908/is-there-a-fix-for-bodycontentimpl-jsp-tag-memory-leak-using-tomcat > * https://issues.apache.org/bugzilla/show_bug.cgi?id=43925 > > Specifically what we see happening is due to large pages being rendered in > JSPs - sometimes in the multi-megabyte range, especially for internal tools. > Apparently, some of the tags we use require buffering the output, versus > sending it directly to the Writer. > > Now, we certainly can simply set LIMIT_BUFFER and forget about it (throw away > any buffers larger than 8k), but this isn't optimal for our application. For > about 75% of our requests, ~8k is the baseline page size, with many larger > than that. So, depending how the data size, we could be reallocating multiple > times, only to throw away the buffer at the end. > > We'd like to propose a different behavior, and are wondering what others > think about it. If it sounds like a good idea, we're certainly willing to > submit patches to implement it. > > --Proposal-- > Instead of a hard-coded DEFAULT_TAG_BUFFER_SIZE of 8k, make it configurable. > In addition, if LIMIT_BUFFER is set, provide another configurable setting > called something like MAX_TAG_BUFFER_SIZE (if not set, defaults to > DEFAULT_TAG_BUFFER_SIZE). This would give applications a more predictable > *range* of memory allocated for tag buffers, while decreasing the frequency > of reallocation. > --/Proposal-- Allowing for a cap on growth and throwing an IOException when it is hit seems like a reasonable precaution to prevent more drastic OOM errors (I've been bitten by this too). Using a threshold rather than simple boolean to control recycling could be helpful. It could easily mimic the current functionality albeit at the cost of backward compatibility. However, I would expect see allocations gradually creep up to this limit across the board so it seems more predictable just to raise the default to that value. > We'd probably set DEFAULT_TAG_BUFFER_SIZE to 16k, and set MAX_TAG_BUFFER_SIZE > to either 64k or 128k, based on real-world performance testing. DEFAULT_TAG_BUFFER_SIZE is only 512 bytes and, AIUI, is set low-ish just to cover the common case where fragments generate small amounts of output. I don't think we should change the defaults for these. > One other, slightly less defined question - is there a reason we're managing > char arrays ourselves, instead of using something like Java's StringBuilder > or Javalution's TextBuilder? StringBuilder would reduce code maintenance, > while something like TextBuilder would also reduce array allocation. Patches are always welcome. Cheers Jeremy
signature.asc
Description: Message signed with OpenPGP using GPGMail
