harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tim Ellison <t.p.elli...@gmail.com>
Subject Re: [classlib][luni] optimized InputStream reader (HARMONY-5522)
Date Sun, 16 Mar 2008 19:51:18 GMT
Alexei Fedotov wrote:
> Tim, thank you for reading and understanding. I like when my code
> passes a review and I am eager to try arguments I've created for
> myself. I thought of your variant and liked it because it would be
> possible to JIT to inline your version of ByteStream.readFully()
> method.
> 
> There is some complication caused with a public API exposed by
> Manifest class: the constructor and read methods accept a general
> InputStream as a source for Manifest. Other applications, such as
> Eclipse, read a file themselves and then call Manifest API directly to
> get few main attributes. There is a better chance that external
> developers would use use documented ByteArrayInputStream to wrap their
> bytes than "our possibly optimized internal child" of that class.
> 
> I also believe that I decrease overall project astonishment by
> encapsulating more complex technique in one place instead of spreading
> less conventional things around. Reflection is hidden in a class with
> conventional API, and no one would question when reading
> ZipFile#getInputStream(ZipEntry) code why he should wrap a resulted
> byte buffer in "our possibly optimized internal child" instead of
> ByteArrayInputStream.
> 
> Finally two these methods may be combined: in addition to
> ByteArrayInputStream one may check in readFully(InputStream) if "our
> possibly optimized internal child" is supplied an use a quick path
> without reflection. I wonder what you and other java gurus think about
> this.

Alexei,

I wrote a version of the class that I was thinking about (without 
reflection) [1].

It has the same #readFully behavior, but if we allow the 
ZipFile#getInputStream method to return a ByteStream then we will get 
the optimizations you talk about.

Take a look and let me know what you think -- then we can take the other 
parts a step at a time.

[1] http://issues.apache.org/jira/secure/attachment/12378007/ByteStream.java

Regards,
Tim

Mime
View raw message