harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alexei Fedotov" <alexei.fedo...@gmail.com>
Subject Re: [classlib][luni] optimized InputStream reader (HARMONY-5522)
Date Mon, 17 Mar 2008 06:29:25 GMT
Tim,
The code is a pretty strong argument. Let me try another two
arguments, or a combined variant as a way to converge with ByteStream
implementation.

Imagine us asking Eclipse guys to wrap their byte arrays containing
manifests with o.a.h.luni.util.ByteBuffer class to add performance for
their code: this is difficult because they probably dislike putting
the Harmony specific classes into their code. My optimization would
work for this case, and, which is more important, this would work for
all cases when a whole byte stream is wrapped with
ByteArrayInputStream. Let me try to envision a bright future of
ByteStream: in addition to readFully() we will add digestFully(),
toUtf8Fully() methods and others which would do the job without
copying even when buffer positions are not at the buffer boundary,
i.e. for arbitrary ByteArrayInoutStream buffer which appears in a
user's code. A completely new argument (though it is not the strongest
one) is that a trick with reflection under PriviledgedAction is
conventional.

So let me summarize:
Your way pluses: + code simplicity, + inlining of readFully (you have
inlined getByteArray without the help of JIT)
My pluses: + opens a way for optimizations of any ByteArrayInputStream
invocation in a user code

What do you think of a combined variant when ByteStream#readFully
checks for both ByteArrayInputStream and ByteStream nature of
InputStream, and continues using reflection for arbitrary
ByteArrayInputStream?


On Sun, Mar 16, 2008 at 10:51 PM, Tim Ellison <t.p.ellison@gmail.com> wrote:
>
> 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
>



-- 
With best regards,
Alexei

Mime
View raw message