Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve HttpOutput.print() #12530

Open
wants to merge 2 commits into
base: jetty-12.1.x
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,10 @@
import java.io.IOException;
import java.io.InputStream;
import java.nio.ByteBuffer;
import java.nio.CharBuffer;
import java.nio.channels.ReadableByteChannel;
import java.nio.channels.WritePendingException;
import java.nio.charset.Charset;
import java.nio.charset.CharsetEncoder;
import java.nio.charset.CoderResult;
import java.nio.charset.CodingErrorAction;
import java.nio.charset.StandardCharsets;
import java.util.concurrent.CancellationException;

import jakarta.servlet.RequestDispatcher;
Expand Down Expand Up @@ -722,6 +719,17 @@ private void checkWritable() throws EofException

@Override
public void write(byte[] b, int off, int len) throws IOException
{
write(b, off, len, null);
}

@FunctionalInterface
private interface IORunnable
{
void run() throws IOException;
}

private void write(byte[] b, int off, int len, IORunnable onComplete) throws IOException
{
if (LOG.isDebugEnabled())
LOG.debug("write(array {})", BufferUtil.toDetailString(ByteBuffer.wrap(b, off, len)));
Expand Down Expand Up @@ -784,6 +792,8 @@ public void write(byte[] b, int off, int len) throws IOException
if (LOG.isDebugEnabled())
LOG.debug("write(array) {} aggregated !flush {}",
stateString(), _aggregate);
if (onComplete != null)
onComplete.run();
return;
}

Expand All @@ -799,8 +809,27 @@ public void write(byte[] b, int off, int len) throws IOException

if (async)
{
IORunnable copy = onComplete;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is copy the right name? It confused me. How about just doOnComplete or runOnComplete?

// Do the asynchronous writing from the callback
new AsyncWrite(b, off, len, last).iterate();
new AsyncWrite(b, off, len, last)
{
@Override
protected void onCompleted(Throwable causeOrNull)
{
if (copy != null)
{
try
{
copy.run();
}
catch (Throwable x)
{
causeOrNull = ExceptionUtil.combine(causeOrNull, x);
}
}
super.onCompleted(causeOrNull);
}
}.iterate();
return;
}

Expand All @@ -820,6 +849,10 @@ public void write(byte[] b, int off, int len) throws IOException
if (len > 0 && !last && len <= _commitSize && len <= maximizeAggregateSpace())
{
BufferUtil.append(byteBuffer, b, off, len);
IORunnable copy = onComplete;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

onComplete = null;
if (copy != null)
copy.run();
onWriteComplete(false, null);
return;
}
Expand Down Expand Up @@ -849,10 +882,16 @@ else if (last && !complete)
channelWrite(BufferUtil.EMPTY_BUFFER, true);
}

IORunnable copy = onComplete;
onComplete = null;
if (copy != null)
copy.run();
onWriteComplete(last, null);
}
catch (Throwable t)
{
if (onComplete != null)
onComplete.run();
onWriteComplete(last, t);
throw t;
}
Expand Down Expand Up @@ -1026,72 +1065,10 @@ private void print(String s, boolean eoln) throws IOException
s = String.valueOf(s);

String charset = _servletChannel.getServletContextResponse().getCharacterEncoding(false);
CharsetEncoder encoder = _encoder.take();
if (encoder == null || !encoder.charset().name().equalsIgnoreCase(charset))
{
encoder = Charset.forName(charset).newEncoder();
encoder.onMalformedInput(CodingErrorAction.REPLACE);
encoder.onUnmappableCharacter(CodingErrorAction.REPLACE);
}
ByteBufferPool pool = _servletChannel.getRequest().getComponents().getByteBufferPool();
RetainableByteBuffer out = pool.acquire((int)(1 + (s.length() + 2) * encoder.averageBytesPerChar()), false);
try
{
CharBuffer in = CharBuffer.wrap(s);
CharBuffer crlf = eoln ? CharBuffer.wrap("\r\n") : null;
ByteBuffer byteBuffer = out.getByteBuffer();
BufferUtil.flipToFill(byteBuffer);

while (true)
{
CoderResult result;
if (in.hasRemaining())
{
result = encoder.encode(in, byteBuffer, crlf == null);
if (result.isUnderflow())
if (crlf == null)
break;
else
continue;
}
else if (crlf != null && crlf.hasRemaining())
{
result = encoder.encode(crlf, byteBuffer, true);
if (result.isUnderflow())
{
if (!encoder.flush(byteBuffer).isUnderflow())
result.throwException();
break;
}
}
else
break;

if (result.isOverflow())
{
BufferUtil.flipToFlush(byteBuffer, 0);
RetainableByteBuffer bigger = pool.acquire(out.capacity() + s.length() + 2, out.isDirect());
BufferUtil.flipToFill(bigger.getByteBuffer());
bigger.getByteBuffer().put(byteBuffer);
out.release();
BufferUtil.flipToFill(bigger.getByteBuffer());
out = bigger;
byteBuffer = bigger.getByteBuffer();
continue;
}

result.throwException();
}

BufferUtil.flipToFlush(byteBuffer, 0);
write(byteBuffer.array(), byteBuffer.arrayOffset(), byteBuffer.remaining());
}
finally
{
out.release();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the above write() call is async, out.release() may be called before the write is done.

encoder.reset();
_encoder.offer(encoder);
}
byte[] bytes = s.getBytes(charset);
IORunnable r = eoln ? () -> write("\r\n".getBytes(StandardCharsets.UTF_8)) : null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use ASCII for \r\n.

The write of this IORunnable could remain pending, so you cannot notify callback until this write is complete.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better yet, use IO.CRLF_BYTES

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and call the variable onComplete

write(bytes, 0, bytes.length, r);
}

/**
Expand Down
Loading