Bug 60146 - Performance issue as CachedResource#getInputStream() is not caching data correctly
Summary: Performance issue as CachedResource#getInputStream() is not caching data corr...
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 8
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 8.5.5
Hardware: PC All
: P2 normal (vote)
Target Milestone: ----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-09-18 17:11 UTC by mohitchugh
Modified: 2016-09-20 10:47 UTC (History)
0 users



Attachments
Patch that potentially fixes the issue in TC8.5.x and trunk. (1.18 KB, patch)
2016-09-18 17:11 UTC, mohitchugh
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description mohitchugh 2016-09-18 17:11:15 UTC
Created attachment 34262 [details]
Patch that potentially fixes the issue in TC8.5.x and trunk.

The problem is with org.apache.catalina.webresources.CachedResource.java

While getContent() is correctly creating a byte cache if the object is small enough, getInputStream() is not. This means that if the caller never calls getContent() before calling getInputStream() no cache is created, and the data is fetched from the file every time.

This turned out to be a big issue in my performance comparison test against Tomcat 6 which was indirectly trying to load a Spring properties file that wasn't getting cached because of this bug. 

I could see Tomcat 8 consistently take 3x the memory and almost 1.5x times the CPU just because the JVM's young-generation space was overwhelmed with dealing with the temporary java.util.zip.inflater objects that got created while Spring (which apparently relies only on getInputStream) was trying to read the properties file.

It seems that the simplest way to fix this issue is to make getInputStream() call getContent() first to establish the cache, rather than assume that getContent() would have been already called. Considering how getContent() is written, I don't think this will lead to any extra overhead.

I did test this one line change and I could see that the memory and CPU utilization in Tomcat 8 dropped back to similar levels to Tomcat 6.

I have only tested against Tomcat 8.5.5 but looking at the code it seems that the issue is also present in trunk.

I am attaching a patch which fixed the issue in my testing. I'm not sure if there's a better fix or if the patch is in the correct format, but I hope the patch at least illustrates the underlying issue.

Thanks.
Comment 1 Mark Thomas 2016-09-20 10:47:55 UTC
Thanks for the report and the patch.

This has been fixed in the following branches:
- 9.0.x for 9.0.0.M11 onwards
- 8.5.x for 8.5.6 onwards
- 8.0.x for 8.0.38 onwards