-----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: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
