-
Notifications
You must be signed in to change notification settings - Fork 327
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove unnecessary calls to CodecPool.returnCompressor/returnDecompresso... #103
Changes from 3 commits
20becc5
efdf80c
cc2c7cd
9ae0ca5
72d71d1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -67,15 +67,19 @@ public class LzopCodec extends LzoCodec { | |
public CompressionOutputStream createOutputStream(OutputStream out) throws IOException { | ||
//get a compressor which will be returned to the pool when the output stream | ||
//is closed. | ||
return createOutputStream(out, getCompressor()); | ||
Compressor compressor = getCompressor(); | ||
OutputStream wrapped = new WrappedOutputStream(out, compressor); | ||
return createOutputStream(wrapped, compressor); | ||
} | ||
|
||
public CompressionOutputStream createIndexedOutputStream(OutputStream out, | ||
DataOutputStream indexOut) | ||
throws IOException { | ||
//get a compressor which will be returned to the pool when the output stream | ||
//is closed. | ||
return createIndexedOutputStream(out, indexOut, getCompressor()); | ||
Compressor compressor = getCompressor(); | ||
OutputStream wrapped = new WrappedOutputStream(out, compressor); | ||
return createIndexedOutputStream(wrapped, indexOut, compressor); | ||
} | ||
|
||
@Override | ||
|
@@ -106,11 +110,56 @@ public CompressionInputStream createInputStream(InputStream in, | |
getConf().getInt(LZO_BUFFER_SIZE_KEY, DEFAULT_LZO_BUFFER_SIZE)); | ||
} | ||
|
||
private static class WrappedInputStream extends InputStream { | ||
private InputStream inputStream; | ||
private Decompressor decompressor; | ||
|
||
public WrappedInputStream(InputStream inputStream, Decompressor decompressor) { | ||
this.inputStream = inputStream; | ||
this.decompressor = decompressor; | ||
} | ||
|
||
@Override | ||
public int read() throws IOException { | ||
return inputStream.read(); | ||
} | ||
|
||
@Override | ||
public void close() throws IOException { | ||
CodecPool.returnDecompressor(decompressor); | ||
inputStream.close(); | ||
} | ||
} | ||
|
||
// Previous versions of the API accidentally added/removed compressor/decompressors from the pool | ||
// when they shouldn't. These classes are kind of a hack to maintain existing behavior, | ||
// while still allowing proper resource management from outside | ||
private static class WrappedOutputStream extends OutputStream { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. when the patch is updated to extend FilterOutputStream, you should implement write(array, offset, len). I don't know why but FilterOutputStream implements this method by writing one char at a time! |
||
OutputStream outputStream; | ||
Compressor compressor; | ||
|
||
public WrappedOutputStream(OutputStream outputStream, Compressor compressor) { | ||
this.outputStream = outputStream; | ||
this.compressor = compressor; | ||
} | ||
|
||
@Override | ||
public void write(int b) throws IOException { | ||
outputStream.write(b); | ||
} | ||
|
||
@Override | ||
public void close() throws IOException { | ||
CodecPool.returnCompressor(compressor); | ||
outputStream.close(); | ||
} | ||
} | ||
|
||
@Override | ||
public CompressionInputStream createInputStream(InputStream in) throws IOException { | ||
// get a decompressor from a pool which will be returned to the pool | ||
// when LzoInputStream is closed | ||
return createInputStream(in, CodecPool.getDecompressor(this)); | ||
Decompressor decompressor = CodecPool.getDecompressor(this); | ||
InputStream inputStream = new WrappedInputStream(in, decompressor); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could be something like:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @rangadi the reason I didn't do that is because then I need to mark the parameter (can't see how, but really just don't know any better) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right. That does not matter. It is just a way to tell Java to carry the reference in the anonymous inner class. |
||
return createInputStream(inputStream, decompressor); | ||
} | ||
|
||
@Override | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should implement
read(byte b[], int off, int len)
, otherwise, reads will be very slow.actually even better is to make it extend FilterInputStream and override only close().
Same for OutputStream.