Alexei Fedotov wrote:
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?
Yep, let's combine them. So we can use the ByteStream in cases when we
create the stream (reading boot JAR files etc.), use reflection to
reach into the ByteArrayInputStream (the use case you have here is
Eclipse), and finally use the InputStream API to read the stream fully
by allocating and copying when all else fails.
Perhaps we can split the ByteStream into the buffer-accessible stream
and read*Fully utility.
Regards,
Tim
