Bug 60596 - Improve performance of DefaultServlet.checkSendfile() when SendFile feature is disabled on a connector
Summary: Improve performance of DefaultServlet.checkSendfile() when SendFile feature i...
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 9
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 9.0.0.M17
Hardware: PC All
: P2 enhancement (vote)
Target Milestone: -----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-01-17 14:34 UTC by Konstantin Kolinko
Modified: 2017-01-21 10:59 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
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.