Bug 60146

Summary: Performance issue as CachedResource#getInputStream() is not caching data correctly
Product: Tomcat 8 Reporter: mohitchugh
Component: CatalinaAssignee: Tomcat Developers Mailing List <dev>
Status: RESOLVED FIXED    
Severity: normal    
Priority: P2    
Version: 8.5.5   
Target Milestone: ----   
Hardware: PC   
OS: All   
Attachments: Patch that potentially fixes the issue in TC8.5.x and trunk.

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