-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Issue #5499 - Reduce buffer allocations and copying from ByteAccumulator #5574
Changes from 6 commits
1f5b446
05dafb8
145bcff
a3c3e24
7bcae99
6e95722
3c44df0
8dc0d99
595d4bf
d75e6de
e0031e0
e7bed39
a1aa5dc
5788fe6
7c46d96
f63a741
2629845
a51b5db
602cd7e
6dce1cb
8aedc50
8b3cffb
41cffa0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,115 @@ | ||
// | ||
// ======================================================================== | ||
// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others. | ||
// ------------------------------------------------------------------------ | ||
// All rights reserved. This program and the accompanying materials | ||
// are made available under the terms of the Eclipse Public License v1.0 | ||
// and Apache License v2.0 which accompanies this distribution. | ||
// | ||
// The Eclipse Public License is available at | ||
// http://www.eclipse.org/legal/epl-v10.html | ||
// | ||
// The Apache License v2.0 is available at | ||
// http://www.opensource.org/licenses/apache2.0.php | ||
// | ||
// You may elect to redistribute this code under either of these licenses. | ||
// ======================================================================== | ||
// | ||
|
||
package org.eclipse.jetty.io; | ||
|
||
import java.io.IOException; | ||
import java.io.OutputStream; | ||
import java.nio.ByteBuffer; | ||
import java.util.ArrayList; | ||
import java.util.List; | ||
|
||
import org.eclipse.jetty.util.BufferUtil; | ||
|
||
public class ByteBufferAccumulator implements AutoCloseable | ||
{ | ||
private static final int MIN_SPACE = 3; | ||
private static final int DEFAULT_BUFFER_SIZE = 1024; | ||
|
||
private final List<ByteBuffer> _buffers = new ArrayList<>(); | ||
private final ByteBufferPool _bufferPool; | ||
|
||
public ByteBufferAccumulator() | ||
{ | ||
this(null); | ||
} | ||
|
||
public ByteBufferAccumulator(ByteBufferPool bufferPool) | ||
{ | ||
_bufferPool = (bufferPool == null) ? new NullByteBufferPool() : bufferPool; | ||
} | ||
|
||
public int getLength() | ||
{ | ||
int length = 0; | ||
for (ByteBuffer buffer : _buffers) | ||
length += buffer.remaining(); | ||
return length; | ||
} | ||
|
||
public ByteBuffer getBuffer() | ||
{ | ||
return getBuffer(DEFAULT_BUFFER_SIZE); | ||
} | ||
|
||
public ByteBuffer getBuffer(int minAllocationSize) | ||
gregw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
ByteBuffer buffer = _buffers.isEmpty() ? BufferUtil.EMPTY_BUFFER : _buffers.get(_buffers.size() - 1); | ||
if (BufferUtil.space(buffer) <= MIN_SPACE) | ||
{ | ||
buffer = _bufferPool.acquire(minAllocationSize, false); | ||
gregw marked this conversation as resolved.
Show resolved
Hide resolved
lorban marked this conversation as resolved.
Show resolved
Hide resolved
|
||
_buffers.add(buffer); | ||
} | ||
|
||
return buffer; | ||
} | ||
|
||
public void copyBytes(byte[] buf, int offset, int length) | ||
{ | ||
copyBuffer(BufferUtil.toBuffer(buf, offset, length)); | ||
} | ||
|
||
public void copyBuffer(ByteBuffer buffer) | ||
gregw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
while (buffer.hasRemaining()) | ||
{ | ||
ByteBuffer b = getBuffer(buffer.remaining()); | ||
int pos = BufferUtil.flipToFill(b); | ||
BufferUtil.put(buffer, b); | ||
BufferUtil.flipToFlush(b, pos); | ||
} | ||
gregw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
public void writeTo(ByteBuffer buffer) | ||
{ | ||
int pos = BufferUtil.flipToFill(buffer); | ||
for (ByteBuffer bb : _buffers) | ||
{ | ||
buffer.put(bb); | ||
} | ||
BufferUtil.flipToFlush(buffer, pos); | ||
} | ||
|
||
public void writeTo(OutputStream out) throws IOException | ||
{ | ||
for (ByteBuffer bb : _buffers) | ||
{ | ||
BufferUtil.writeTo(bb, out); | ||
} | ||
} | ||
|
||
@Override | ||
public void close() | ||
{ | ||
for (ByteBuffer buffer : _buffers) | ||
{ | ||
_bufferPool.release(buffer); | ||
} | ||
_buffers.clear(); | ||
} | ||
gregw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,144 @@ | ||
// | ||
// ======================================================================== | ||
// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others. | ||
// ------------------------------------------------------------------------ | ||
// All rights reserved. This program and the accompanying materials | ||
// are made available under the terms of the Eclipse Public License v1.0 | ||
// and Apache License v2.0 which accompanies this distribution. | ||
// | ||
// The Eclipse Public License is available at | ||
// http://www.eclipse.org/legal/epl-v10.html | ||
// | ||
// The Apache License v2.0 is available at | ||
// http://www.opensource.org/licenses/apache2.0.php | ||
// | ||
// You may elect to redistribute this code under either of these licenses. | ||
// ======================================================================== | ||
// | ||
|
||
package org.eclipse.jetty.io; | ||
|
||
import java.io.IOException; | ||
import java.io.OutputStream; | ||
import java.nio.ByteBuffer; | ||
|
||
import org.eclipse.jetty.util.BufferUtil; | ||
|
||
/** | ||
* This class implements an output stream in which the data is written into a list of ByteBuffer, | ||
* the buffer list automatically grows as data is written to it, the buffers are taken from the | ||
* supplied {@link ByteBufferPool} or freshly allocated if one is not supplied. | ||
* | ||
* Designed to mimic {@link java.io.ByteArrayOutputStream} but with better memory usage, and less copying. | ||
*/ | ||
public class ByteBufferOutputStream2 extends OutputStream | ||
{ | ||
private final ByteBufferAccumulator _accumulator; | ||
private final ByteBufferPool _bufferPool; | ||
lachlan-roberts marked this conversation as resolved.
Show resolved
Hide resolved
|
||
private ByteBuffer _combinedByteBuffer; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there really a need to remember the combined ByteBuffer? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to remember it so that we release it when There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it is wrong for this class to manage the releasing of the combined buffer. Isn't that mostly passed along a chain and will be released when processing is complete. I still lean towards There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this behaviour can be implemented in
|
||
private int _size = 0; | ||
|
||
public ByteBufferOutputStream2() | ||
{ | ||
this(null); | ||
} | ||
|
||
public ByteBufferOutputStream2(ByteBufferPool bufferPool) | ||
{ | ||
_bufferPool = (bufferPool == null) ? new NullByteBufferPool() : bufferPool; | ||
_accumulator = new ByteBufferAccumulator(bufferPool); | ||
lachlan-roberts marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
/** | ||
* Get an aggregated content written to the OutputStream in a ByteBuffer. | ||
* @return the content in a ByteBuffer. | ||
*/ | ||
public ByteBuffer toByteBuffer() | ||
gregw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
int length = _accumulator.getLength(); | ||
if (length == 0) | ||
return BufferUtil.EMPTY_BUFFER; | ||
|
||
if (_combinedByteBuffer != null && length == _combinedByteBuffer.remaining()) | ||
lachlan-roberts marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return _combinedByteBuffer; | ||
|
||
ByteBuffer buffer = _bufferPool.acquire(_size, false); | ||
_accumulator.writeTo(buffer); | ||
if (_combinedByteBuffer != null) | ||
{ | ||
_bufferPool.release(_combinedByteBuffer); | ||
_combinedByteBuffer = buffer; | ||
} | ||
|
||
return buffer; | ||
} | ||
|
||
/** | ||
* Get an aggregated content written to the OutputStream in a byte array. | ||
* @return the content in a byte array. | ||
*/ | ||
public byte[] toByteArray() | ||
{ | ||
int length = _accumulator.getLength(); | ||
if (length == 0) | ||
return new byte[0]; | ||
|
||
byte[] bytes = new byte[_size]; | ||
ByteBuffer buffer = BufferUtil.toBuffer(bytes); | ||
_accumulator.writeTo(buffer); | ||
return bytes; | ||
} | ||
|
||
public int size() | ||
{ | ||
return _accumulator.getLength(); | ||
} | ||
|
||
@Override | ||
public void write(int b) | ||
{ | ||
write(new byte[]{(byte)b}, 0, 1); | ||
} | ||
|
||
@Override | ||
public void write(byte[] b, int off, int len) | ||
{ | ||
write(BufferUtil.toBuffer(b, off, len)); | ||
} | ||
|
||
public void write(ByteBuffer buffer) | ||
{ | ||
gregw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
while (buffer.hasRemaining()) | ||
gregw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
ByteBuffer lastBuffer = _accumulator.getBuffer(buffer.remaining()); | ||
int pos = BufferUtil.flipToFill(lastBuffer); | ||
_size += BufferUtil.put(buffer, lastBuffer); | ||
BufferUtil.flipToFlush(lastBuffer, pos); | ||
} | ||
} | ||
|
||
public void writeTo(OutputStream out) throws IOException | ||
{ | ||
_accumulator.writeTo(out); | ||
} | ||
|
||
@Override | ||
public void close() | ||
{ | ||
if (_combinedByteBuffer != null) | ||
{ | ||
_bufferPool.release(_combinedByteBuffer); | ||
_combinedByteBuffer = null; | ||
} | ||
|
||
_accumulator.close(); | ||
_size = 0; | ||
} | ||
|
||
@Override | ||
public synchronized String toString() | ||
{ | ||
return String.format("%s@%x{size=%d, bufferPool=%s, byteAccumulator=%s}", getClass().getSimpleName(), | ||
hashCode(), _size, _bufferPool, _accumulator); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
// | ||
// ======================================================================== | ||
// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others. | ||
// ------------------------------------------------------------------------ | ||
// All rights reserved. This program and the accompanying materials | ||
// are made available under the terms of the Eclipse Public License v1.0 | ||
// and Apache License v2.0 which accompanies this distribution. | ||
// | ||
// The Eclipse Public License is available at | ||
// http://www.eclipse.org/legal/epl-v10.html | ||
// | ||
// The Apache License v2.0 is available at | ||
// http://www.opensource.org/licenses/apache2.0.php | ||
// | ||
// You may elect to redistribute this code under either of these licenses. | ||
// ======================================================================== | ||
// | ||
|
||
package org.eclipse.jetty.io; | ||
|
||
import java.nio.ByteBuffer; | ||
|
||
import org.eclipse.jetty.util.BufferUtil; | ||
|
||
public class NullByteBufferPool implements ByteBufferPool | ||
{ | ||
@Override | ||
public ByteBuffer acquire(int size, boolean direct) | ||
{ | ||
if (direct) | ||
return BufferUtil.allocateDirect(size); | ||
else | ||
return BufferUtil.allocate(size); | ||
} | ||
|
||
@Override | ||
public void release(ByteBuffer buffer) | ||
{ | ||
BufferUtil.clear(buffer); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This length accumulation could potentially overflow
Integer.MAX_VALUE
. We should handle this by either returning a long from here, or detecting the integer overflow and throwing an exception.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the accumulation needs to eventually fit into a single buffer, the int should do with overflow detection elsewhere.
An ultimate buffer abstraction could allow for chains of buffers that can be gather written for a long length, but not this abstraction