RFR: 8319531: FileServerHandler::discardRequestBody could be improved

Daniel Fuchs dfuchs at openjdk.org
Fri Nov 10 15:53:02 UTC 2023


On Fri, 10 Nov 2023 15:26:06 GMT, Darragh Clarke <dclarke at openjdk.org> 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

Would be good to have @Michael-Mc-Mahon input on this (on both your proposed fix and my suggestion)

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;
    }

-------------

PR Review: https://git.openjdk.org/jdk/pull/16616#pullrequestreview-1725058054
PR Review Comment: https://git.openjdk.org/jdk/pull/16616#discussion_r1389564869


More information about the net-dev mailing list