On 2/6/18, 8:28 AM, Roger Riggs wrote:
Hi Sherman,
On 2/5/2018 9:00 PM, Xueming Shen wrote:
Hi,
Please help review the change for JDK-8164278.
issue: https://bugs.openjdk.java.net/browse/JDK-8164278
webrev: http://cr.openjdk.java.net/~sherman/8164278/webrev
Are the reentrant locks necessary? concurrent reads from streams are
not usually
synchronized so its the caller that need to synchronize.
If locks are necessary, why no lock for the EncOutputStream buffer?
Thanks Roger,
I don't really care the "correctnesss" of the de/coding via the
in/output stream
under concurrent access. As you said the caller should take the
responsibility
for that. In case of DecInputStream there is risk that the "pos" might
be updated
out of the buffer boundary and trigger exception, if under concurrent
access.
It's not the case for EncOutputStream. But yes, let me see if I can get
away with
this via local copy of the pos/cnt.
809: Can the buffer byte array be sized based on linemax? The field
declaration should
be with the other fields at the top of the file.
sure possible with some calculation.
848: checkNewline compares == with linemax; that works when each byte
is counted separately
It seems like it would safer if it was ">=".
updated.
957: can sbBuf be shared with lbuf?
probably not. lbuf is for the bytes from underlying input stream. sbBuf
is to
store/get the first byte of the decoded result.
However, there is a side-effect of adding a buffering mechanism
into DecInputStream. The
current implementation read bytes from the underlying stream one
by one, it never reads
more bytes than it needs, which means it should/is supposed to
just stop at the last byte
that it needs to decode, when there is "=" present in the stream.
With buffering, it's possible more bytes (after "=", which
indicates "end of base64 stream") might
be read/consumed in and buffered. A concern? if this is indeed a
concern, the only alternative
might be to add a separate method to support this
"faster-buffered-decoder"?
How much buffering is needed to speed it up? Can the mark/reset
functions of the underlying
stream be used to backup the stream if it overshoots?
If mark and reset are not supported then read 1 byte at a time.
will take a look at the mark/reset possibility. maybe I will put
DecInputStream optimization to a
separate rfe.
-Sherman
