Bug 60596

Summary: Improve performance of DefaultServlet.checkSendfile() when SendFile feature is disabled on a connector
Product: Tomcat 9 Reporter: Konstantin Kolinko <knst.kolinko>
Component: CatalinaAssignee: Tomcat Developers Mailing List <dev>
Status: RESOLVED FIXED    
Severity: enhancement    
Priority: P2    
Version: 9.0.0.M17   
Target Milestone: -----   
Hardware: PC   
OS: All   

Description Konstantin Kolinko 2017-01-17 14:34:01 UTC
Reviewing SendFile support in DefaultServlet,
the feature is implemented with the following code in current trunk:

// method DefaultServlet.checkSendfile()

        if (sendfileSize > 0
            && resource.isFile()
            && length > sendfileSize
            && (resource.getCanonicalPath() != null)
            && (Boolean.TRUE.equals(request.getAttribute(Globals.SENDFILE_SUPPORTED_ATTR)))
            && (request.getClass().getName().equals("org.apache.catalina.connector.RequestFacade"))
            && (response.getClass().getName().equals("org.apache.catalina.connector.ResponseFacade"))) {
            request.setAttribute(Globals.SENDFILE_FILENAME_ATTR, resource.getCanonicalPath());


Notes:

1. It is better to check for request.getAttribute(Globals.SENDFILE_SUPPORTED_ATTR) flag first.

Calling resource.isFile() and resource.getCanonicalPath() incurs I/O processing that is not needed when send file feature is disabled on a Connector.

2. resource.getCanonicalPath() is calculated twice. First time to check that it is not null, second time to set SENDFILE_FILENAME_ATTR attribute.


In general, all this activity will be followed by File I/O, so performance improvement may be small compared to the rest of I/O.

Duplicate evaluation of getCanonicalPath() is not a matter for Tomcat 7 and earlier, as in the old JNDI implementation of resources the value of canonical path is cached (see org.apache.naming.resources.FileResourceAttributes.getCanonicalPath()).
Comment 1 Konstantin Kolinko 2017-01-21 10:53:40 UTC
Fixed in Tomcat 9 by r1779718 to be in 9.0.0.M18.
Fixed in Tomcat 8.5 by r1779719 to be in 8.5.12.
Fixed in Tomcat 8.0 by r1779721 to be in 8.0.42.
Comment 2 Konstantin Kolinko 2017-01-21 10:59:23 UTC
Not an issue for Tomcat 7 and earlier, as
- existence check (entry.resource != null) does not involve I/O
- FileResourceAttributes.getCanonicalPath() caches its value

Closing as FIXED.