Avoid large allocations of temporary buffers in BuferUtil

[SerCeMan]

Hi, Jetty Maintainers!

We have recently noticed that our jetty instances suffer from a large number of young GC pauses. The allocation profiler pointed out that a large number of allocations is occurring in BufferUtil#writeTo.

In our use-case, we're receiving a large number of WebSocket messages from many connections but most of them are very small, typically tiny keep-alive frames. The change in this PR can help to avoid allocating 4K temporary arrays when only a few bytes need to be copied. This could potentially be optimised even further with a specialised implementation for o.e.j.u.ByteArrayOutputStream2, however, the change won't be as simple.

Additionally, I ran o.e.j.u.BufferUtilTest#testWriteToMicrobenchmark with capacity = 1024, iterations = 1000 and it showed a measurable difference in the number of young gc cycles.

Before:

2020-07-15 13:21:50.986:WARN:oeju.BufferUtilTest:main: overall average: 3ms

[0.500s][info   ][gc,heap,exit ] Heap
[0.500s][info   ][gc,heap,exit ]  garbage-first heap   total 2097152K, used 79872K [0x0000000780000000, 0x0000000800000000)
[0.500s][info   ][gc,heap,exit ]   region size 1024K, 79 young (80896K), 0 survivors (0K)
[0.500s][info   ][gc,heap,exit ]  Metaspace       used 12797K, capacity 13316K, committed 13696K, reserved 1060864K
[0.500s][info   ][gc,heap,exit ]   class space    used 1321K, capacity 1537K, committed 1664K, reserved 1048576K

After:

2020-07-15 13:20:21.798:WARN:oeju.BufferUtilTest:main: overall average: 2ms

[0.497s][info   ][gc,heap,exit ] Heap
[0.497s][info   ][gc,heap,exit ]  garbage-first heap   total 2097152K, used 50176K [0x0000000780000000, 0x0000000800000000)
[0.497s][info   ][gc,heap,exit ]   region size 1024K, 50 young (51200K), 0 survivors (0K)
[0.497s][info   ][gc,heap,exit ]  Metaspace       used 12788K, capacity 13316K, committed 13696K, reserved 1060864K
[0.497s][info   ][gc,heap,exit ]   class space    used 1321K, capacity 1537K, committed 1664K, reserved 1048576K

As a reference, previous a similar change was introduced in #4561. I'm also happy to open a separate PR for the 10.0.x branch if it sounds good to you.

[gregw]

Thanks for this. I'm reviewing now.
Meanwhile, do you have a stack trace showing where writeTo is being used from?

[gregw]

This is a good simple improvement. However I am concerned that this method is being called frequently enough with a direct buffer to cause a noticeable memory issue. A better way would be to use the direct buffer directly, or not make it a direct buffer in the first place! So we can accept this change, but more work to be done here.

[gregw]

Actually slight change would be better. No need to assign the buffer in the loop:

Suggested change

	byte[] bytes = null;
	while (buffer.hasRemaining())
	{
	int byteCountToWrite = Math.min(buffer.remaining(), TEMP_BUFFER_SIZE);
	if (bytes == null)
	bytes = new byte[byteCountToWrite];
	byte[] bytes = new byte[Math.min(buffer.remaining(), TEMP_BUFFER_SIZE)];
	while (buffer.hasRemaining())
	{

[gregw]

@sbordet @lorban , The HttpContentRangeWriter is preferring a direct buffer, but ByteBufferRangeWriter always does a writeTo, causing a copy to a temp buffer. This is a very suboptimal approach!

[SerCeMan]

Hey, @gregw! Thank you for the review, the feedback is addressed in 99d1926 as suggested.

Meanwhile, do you have a stack trace showing where writeTo is being used from?

You might have already found out, but it's called from
https://github.com/eclipse/jetty.project/blob/6d88f966e1bfe910f03ac1179ee84877c47d01e2/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/message/SimpleBinaryMessage.java#L61

[gregw]

LGTM - although we will still review in #5048

[SerCeMan]

Thanks for merging! Do I need to open a separate PR into the 10.0.x branch?

[gregw]

@SerCeMan I'll merge through to 10, then 11