On Fri, 10 Nov 2023 15:44:23 GMT, Daniel Fuchs <[email protected]> wrote:
>> **Problem** >> `discardRequestBody` calls `InputStream::readAllBytes` to read and discard >> all the request body bytes. This is somewhat wasteful as they can instead be >> skipped. >> >> **Changes** >> - Updated `FileServerHandler::discardRequestBody` to use `InputStream::skip`. >> - Created skip in `LeftOverInputStream` based on >> [InputStream](https://github.com/openjdk/jdk/blob/c9657cad124d2be10b8d6006d0ca9a038b1c5945/src/java.base/share/classes/java/io/InputStream.java#L540). >> This gets used by `FixedLengthInputStream` and `ChunkedInputStream` which >> previously had been using the skip implementation from `FilteredInputStream` >> which would have caused blocking. >> >> - Also made a minor change to `LeftOverInputStream::Drain` to change bufSize. >> >> >> I ran test tiers 1-3 and all tests are passing with these changes > > src/jdk.httpserver/share/classes/sun/net/httpserver/LeftOverInputStream.java > line 115: > >> 113: byte[] skipBuffer = new byte[size]; >> 114: while (remaining > 0) { >> 115: nr = readImpl(skipBuffer, 0, (int)Math.min(size, >> remaining)); > > I wonder if we should import lines 135-137 just before line 115: > > Suggestion: > > if (server.isFinishing()) { > break; > } > nr = readImpl(skipBuffer, 0, (int)Math.min(size, remaining)); > > > and then implement drain simply as: > > > public boolean drain (long l) throws IOException { > while (l > 0) { > long skip = skip(l); > if (skip <= 0) break; // might return 0 if isFinishing or EOF > l -= skip; > } > return eof; > } Yeah, it seems better that drain() would call the skip() that's been implemented. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16616#discussion_r1389685437
