You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@tomcat.apache.org by "Mathias P.W Nilsson" <ma...@snyltarna.se> on 2008/06/24 00:19:05 UTC
Faster resource servlet
Hi!
I have made a resource servlet to handle static content outside of tomcat,
wicket. It looks like this
package se.edgesoft.hairless.web.resource;
import java.io.DataInputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import javax.servlet.ServletConfig;
import javax.servlet.ServletContext;
import javax.servlet.ServletException;
import javax.servlet.ServletOutputStream;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.springframework.web.context.support.WebApplicationContextUtils;
import se.edgesoft.hairless.application.HairlessApplicationSettings;
/**
* Resource servlet for getting images and flash movies
* @author Mathias Nilsson
*
*/
public class FileResourceServlet extends HttpServlet {
private static final long serialVersionUID = 1L;
private static ServletConfig config;
File file;
protected Object getBean(String name) {
Object obj =
WebApplicationContextUtils.getWebApplicationContext(config.getServletContext()).getBean(name);
return obj;
}
public void destroy() {
config = null;
}
public ServletConfig getServletConfig() {
return config;
}
public String getServletInfo() {
return "Resource servlet for Hairless";
}
public void init(ServletConfig servletConfig ) throws ServletException {
config = servletConfig;
}
public HairlessApplicationSettings getHairlessApplicationSettings(){
return (HairlessApplicationSettings)getBean(
"hairlessApplicationSettings" );
}
@Override
protected void doGet(HttpServletRequest request, HttpServletResponse
response) throws ServletException, IOException {
try {
file = new File(
getHairlessApplicationSettings().getFileResourcePath() ,
request.getRequestURI().replace( request.getContextPath(), "" ) );
ServletContext context = getServletConfig().getServletContext();
String mimetype = context.getMimeType( file.getAbsolutePath()
);
response.setContentType( (mimetype != null) ? mimetype :
"application/octet-stream" );
response.setContentLength( (int)file.length() );
int length = 0;
ServletOutputStream op = response.getOutputStream();
byte[] bbuf = new byte[1024];
DataInputStream in = new DataInputStream(new
FileInputStream(file));
while ((in != null) && ((length = in.read(bbuf)) != -1))
{
op.write(bbuf,0,length);
}
in.close();
op.flush();
op.close();
} catch (Exception e) {
//e.printStackTrace();
}
}
}
Maybe I'm missing something but I think it is a little slow. Should I take
something more into consideration?
--
View this message in context: http://www.nabble.com/Faster-resource-servlet-tp18079759p18079759.html
Sent from the Tomcat - User mailing list archive at Nabble.com.
---------------------------------------------------------------------
To start a new topic, e-mail: users@tomcat.apache.org
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org
Re: Faster resource servlet
Posted by Christopher Schultz <ch...@christopherschultz.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
David,
David Fisher wrote:
| Incrementally try larger buffer sizes.
Or, better yet, allow the buffer size to be configurable as deployment
time as an init-param.
| Also,
|
|> response.setContentLength( (int)file.length() );
|
| May be expensive, see how long it takes.
It should be super fast, since you're just looking at filesystem
metadata. The real problem with that line is that it casts a long to an
int, and could lose data.
Instead of using response.setContentLength, you should use:
response.setHeader("Content-Length", String.valueOf(file.length()));
This will allow your servlet to work with files larger than 2GiB.
A couple of other things I'd like to mention:
0) it would be good to divorce the servlet from Spring and other
packages so it can be deployed without dependencies. Whatever
"hairlessApplicationSettings" is could easily be replaced with
init-param settings
1) Don't bother setting serialVersionId. Objects of this class will
never be instantiated.
2) 'config' should not be static.
3) Having "file" be a class-level member is has race conditions built
right into it. This should be a variable local to the doGet method.
4) Calling String.replace to remove the request URI's context prefix is
an expensive operation that is not necessary: just use substring instead.
5) Why are you using a DataInputStream instead of just a bare
InputStream? DataInputStream is used to read serialized Java objects
from a stream. It's probably performing a lot of extra operations that
you simply do not need. I would use a BufferedInputStream instead of a
DataInputStream.
6) Don't check for a null input stream every time you go through the
loop. Check for null once, and then don't check again. Unless you fear
that your pointers will suddenly null-out, it's unnecessary.
7) You need to significantly improve exception handling. Nowhere in your
code are the target files checked for existence and 404 errors returned
or anything like that. Similarly, you need to ensure that resource leaks
do not occur when exceptions do: you don't have any "finally" clause
that closes any open file handles or anything. This is a definite must
for anything considered even remotely production-quality.
8) Only catch exceptions that you can actually do anything about (or are
required to by the compiler). Your code will catch and silently ignore
even things like RuntimeExceptions (like NPEs, etc.) and you really want
to know about those.
Hope those notes help,
- -chris
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iEYEARECAAYFAkhgaMYACgkQ9CaO5/Lv0PBTcQCeNnXHQQeWpZaRn6r8u7C1oqsK
c3EAni1vzBr3qPrzpGoyxOxYhW8bG7z6
=fSw8
-----END PGP SIGNATURE-----
---------------------------------------------------------------------
To start a new topic, e-mail: users@tomcat.apache.org
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org
Re: Faster resource servlet
Posted by David Fisher <df...@jmlafferty.com>.
Incrementally try larger buffer sizes.
> byte[] bbuf = new byte[1024];
byte[] bbuf = new byte[2048];
byte[] bbuf = new byte[4096];
byte[] bbuf = new byte[8192];
byte[] bbuf = new byte[16834];
Then narrow in on the size. I suspect that 8192 (or there abouts will
be best.
Also,
> response.setContentLength( (int)file.length() );
May be expensive, see how long it takes.
Regards,
Dave
On Jun 23, 2008, at 5:19 PM, Mathias P.W Nilsson wrote:
>
> Hi!
>
> I have made a resource servlet to handle static content outside of
> tomcat,
> wicket. It looks like this
>
> package se.edgesoft.hairless.web.resource;
>
> import java.io.DataInputStream;
> import java.io.File;
> import java.io.FileInputStream;
> import java.io.IOException;
>
> import javax.servlet.ServletConfig;
> import javax.servlet.ServletContext;
> import javax.servlet.ServletException;
> import javax.servlet.ServletOutputStream;
> import javax.servlet.http.HttpServlet;
> import javax.servlet.http.HttpServletRequest;
> import javax.servlet.http.HttpServletResponse;
>
> import org.springframework.web.context.support.WebApplicationContextUtils
> ;
>
> import se.edgesoft.hairless.application.HairlessApplicationSettings;
>
> /**
> * Resource servlet for getting images and flash movies
> * @author Mathias Nilsson
> *
> */
> public class FileResourceServlet extends HttpServlet {
> private static final long serialVersionUID = 1L;
> private static ServletConfig config;
> File file;
>
> protected Object getBean(String name) {
> Object obj =
> WebApplicationContextUtils
> .getWebApplicationContext(config.getServletContext()).getBean(name);
> return obj;
> }
> public void destroy() {
> config = null;
> }
>
> public ServletConfig getServletConfig() {
> return config;
> }
>
> public String getServletInfo() {
> return "Resource servlet for Hairless";
> }
>
> public void init(ServletConfig servletConfig ) throws
> ServletException {
> config = servletConfig;
>
> }
>
>
>
> public HairlessApplicationSettings
> getHairlessApplicationSettings(){
> return (HairlessApplicationSettings)getBean(
> "hairlessApplicationSettings" );
> }
>
> @Override
> protected void doGet(HttpServletRequest request,
> HttpServletResponse
> response) throws ServletException, IOException {
> try {
> file = new File(
> getHairlessApplicationSettings().getFileResourcePath() ,
> request.getRequestURI().replace( request.getContextPath(), "" ) );
> ServletContext context =
> getServletConfig().getServletContext();
> String mimetype =
> context.getMimeType( file.getAbsolutePath()
> );
> response.setContentType( (mimetype != null) ? mimetype :
> "application/octet-stream" );
> response.setContentLength( (int)file.length() );
> int length = 0;
> ServletOutputStream op = response.getOutputStream();
>
>
> byte[] bbuf = new byte[1024];
> DataInputStream in = new DataInputStream(new
> FileInputStream(file));
>
> while ((in != null) && ((length = in.read(bbuf)) !=
> -1))
> {
> op.write(bbuf,0,length);
> }
>
> in.close();
> op.flush();
> op.close();
>
> } catch (Exception e) {
> //e.printStackTrace();
> }
> }
>
>
> }
>
>
> Maybe I'm missing something but I think it is a little slow. Should
> I take
> something more into consideration?
> --
> View this message in context: http://www.nabble.com/Faster-resource-servlet-tp18079759p18079759.html
> Sent from the Tomcat - User mailing list archive at Nabble.com.
>
>
> ---------------------------------------------------------------------
> To start a new topic, e-mail: users@tomcat.apache.org
> To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: users-help@tomcat.apache.org
>
>
---------------------------------------------------------------------
To start a new topic, e-mail: users@tomcat.apache.org
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org