Commit Graph

111 Commits

Author SHA1 Message Date
Chris Vest
8cdcfd53c9
Merge pull request #23 from netty/read-only
Add support for read-only buffers
2021-01-05 17:27:10 +01:00
Chris Vest
617d9ccef1 Add support for read-only buffers
Motivation:
There are cases where you want a buffer to be "constant."
Buffers are inherently mutable, but it's possible to block off write access to the buffer contents.
This doesn't make it completely safe to share the buffer across multiple threads, but it does catch most races that could occur.

Modification:
Add a method to Buf for toggling read-only mode.
When a buffer is read-only, the write accessors throw exceptions when called.
In the MemSegBuf, this is implemented by having separate read and write references to the underlying memory segment.
In a read-only buffer, the write reference is redirected to point to a closed memory segment, thus preventing all writes to the memory backing the buffer.

Result:
It is now possible to make buffers read-only.
Note, however, that it is also possible to toggle a read-only buffer back to writable.
We need that in order for buffer pools to be able to fully reset the state of a buffer, regardless of the buffer implementation.
2021-01-05 16:53:21 +01:00
Chris Vest
f862220c4a Readme file updates 2021-01-05 12:51:17 +01:00
Chris Vest
3b1743c7fd Javadoc corrections for buffer accessor methods
Fixes #22
2021-01-05 12:24:46 +01:00
Chris Vest
d72bdb7fc5
Merge pull request #21 from netty/no-confinement
Remove thread-confinement of Buffers
2021-01-05 12:18:31 +01:00
Chris Vest
41b3c02812 Remove thread-confinement of Buffers
Motivation:
Thread-confinement ends up being too confusing to code for, and also prevents some legitimate use cases.
Additionally, thread-confinement exposed implementation specific behavioural differences of buffers, where we would ideally like all buffers to always behave the same, regardless of implementation.

Modification:
All MemorySegment based buffers now always use shared segments.
For heap-based segments, we avoid the overhead associated with the closing of shared segments, by just not closing them, and instead just leave the whole thing for the GC to deal with.

Result:
Buffers can now always be accessed from multiple different threads at the same time.
2020-12-30 15:42:33 +01:00
Chris Vest
9afad3a578
Merge pull request #20 from netty/always-cleaner
Buffers always have a cleaner attached
2020-12-30 14:51:29 +01:00
Chris Vest
dc281c704c Buffers always have a cleaner attached
Motivation:
Although having a cleaner attached adds a bit of overhead when allocating or closing buffers,
it is more important to make our systems, libraries and frameworks misuse resistant and safe by default.

Modification:
Remove the ability to allocate a buffer that does not have a cleaner attached.
Reference counting and the ability to explicitly release memory remains.
This just makes sure that we always have a safety net to fall back on.

Result:
This will make systems less prone to crashes through running out of memory, native or otherwise, even in the face of true memory leaks.
(Leaks through retained strong references cannot be fixed in any way)
2020-12-17 16:13:43 +01:00
Chris Vest
1013c33c3a
Merge pull request #19 from netty/cleaner-close-bench
Add a benchmark that explore the overhead of always attaching a cleaner to buffers
2020-12-17 15:43:53 +01:00
Chris Vest
5697af4be3 Add a benchmark that explore the overhead of always attaching a cleaner to buffers
Looks like the overhead is not too bad, so I think we can just always do that:

```
Benchmark                       (workload)  Mode  Cnt  Score   Error  Units
explicitPooledClose                  light  avgt  150  1,094 ± 0,017  us/op
pooledWithCleanerExplicitClose       light  avgt  150  1,181 ± 0,009  us/op
```
2020-12-17 15:24:28 +01:00
Chris Vest
6697840a34
Merge pull request #18 from netty/compact
Buffer Compaction
2020-12-17 15:18:37 +01:00
Chris Vest
4036dac84d Add a flag to allow Buf.ensureWritable to compact buffer
Motivation:
The main use case with Buf.compact is in conjunction with ensureWritable.
It turns out we can get a simpler API, and faster methods, by combining those two operations, because it allows us to relax some guarantees and skip some steps in certain cases, which wouldn't be as neat or clean if they were two separate steps.

Modification:
Add a new Buf.ensureWritable method, which takes an allowCompaction argument.
In MemSegBuf, we can just delegate to compact() when applicable.
In CompositeBuf, we can sometimes get away with just reorganising the bufs array.

Result:
We can now do ensureWritable without allocating in some cases, and this can in particular make the operation faster for CompositeBuf.
2020-12-17 12:29:55 +01:00
Chris Vest
252ac38305 Simplify MemSegBuf.compact implementation 2020-12-17 12:29:55 +01:00
Chris Vest
0f303c7971 Add a Buf.compact method
Motivation:
Compaction makes more space available at the end of a buffer, by discarding bytes at the beginning that have already been processed.

Modification:
Add a copying compact method to Buf.

Result:
It is now possible to discard read bytes by calling `compact()`.
2020-12-17 12:29:55 +01:00
Chris Vest
cc685c0516
Merge pull request #17 from netty/buf-holder
Add BufHolder and BufRef helper classes
2020-12-17 11:45:11 +01:00
Chris Vest
008c5ed6ec Make BufHolder protected methods final if they're not meant to be overwritten 2020-12-16 17:06:22 +01:00
Chris Vest
6b91751bea Small polishing that addresses PR comments 2020-12-16 12:31:49 +01:00
Chris Vest
f83e7fa618 Add BufHolder and BufRef helper classes
Motivation:
There are many use cases where other objects will have fields that are buffers.
Since buffers are reference counted, their life cycle needs to be managed carefully.

Modification:
Add the abstract BufHolder, and the concrete sub-class BufRef, as neat building blocks for building other classes that contain field references to buffers.

The behaviours of closed/sent buffers have also been specified in tests, and tightened up in the code.

Result:
It is now easier to create classes/objects that wrap buffers.
2020-12-14 14:22:37 +01:00
Chris Vest
ab95abac25 The make clean command now also cleans up after failed build commands 2020-12-11 12:10:04 +01:00
Chris Vest
02eb4286fa Better method names and javadocs in RcSupport 2020-12-11 12:09:32 +01:00
Chris Vest
3cddd2b0b2
Merge pull request #16 from netty/bifurcate
Add a Buf.bifurcate method
2020-12-10 16:07:09 +01:00
Chris Vest
cccec1ae4c Add two more tests for interactions between bifurcation and send 2020-12-10 14:27:45 +01:00
Chris Vest
bb2264ac5b Address review comments on bifurcate PR 2020-12-10 12:51:18 +01:00
Chris Vest
b749106c0c Add a Buf.bifurcate method
Motivation:
There are use cases that involve accumulating data into a buffer, then carving out prefix slices and sending them off on their own journey for further processing.

Modification:
Add a Buf.bifurcate API, that split a buffer, and its ownership, in two.
Internally, the API will inject and maintain an atomically reference counted Drop instance, so that the original memory segment is not released until all bifurcated parts are closed.
This works particularly well for composite buffers, where only the buffer (if any) wherein the bifurcation point lands, will actually have its memory split. A composite buffer can otherwise just crack its buffer array in two.

Result:
We now have a safe way of breaking the single ownership of some memory into multiple parts, that can be sent and owned independently.
2020-12-10 10:29:31 +01:00
Chris Vest
deeea157c0
Merge pull request #11 from netty/byte-cursor
Turn ByteIterator into ByteCursor
2020-12-09 17:23:23 +01:00
Chris Vest
4960e8b3e7 Make ByteCursors from CompositeBuf slightly faster 2020-12-09 11:03:30 +01:00
Chris Vest
a7701c04b5 Update ByteCursor method names and related javadocs 2020-12-09 11:03:30 +01:00
Chris Vest
b2d0231f27 Specify the behaviour of ByteCursor.getByte and getLong, when relevant next* methods haven't been called 2020-12-09 11:02:51 +01:00
Chris Vest
6cc49c1c62 Turn ByteIterator into ByteCursor
Motivation:
Cursors are better than iterators in that they only need to check boundary conditions once per iteration, when processed in a loop.
This should make them easier for the compiler to optimise.

Modification:
Change the ByteIterator to a ByteCursor. The API is almost the same, but with a few subtle differences in semantics.
The primary difference is that the cursor movement and boundary condition checking and position movement happen at the same time, and do not need to occur when the values are fetched out of the cursor.
An iterator, on the other hand, needs to throw an exception if "next" is called too many times.

Result:
Simpler code, and hopefully faster code as well.
2020-12-09 11:02:51 +01:00
Chris Vest
3aeebdd058
Merge pull request #12 from netty/extend-composite
Make it possible to extend composite buffers after creation
2020-12-08 19:25:32 +01:00
Chris Vest
e8a38185bb
Merge pull request #15 from netty/fix-drop-gate
Fix drop race with Cleaner
2020-12-07 17:56:02 +01:00
Chris Vest
2ce8c7dc18 Fix drop race with Cleaner
Motivation:
We were seeing rare test failures where a cleaner had raced to close a memory segment we were using or closing.
The cause was that a single MemorySegment ended up used in multiple Buf instances.
When the SizeClassedMemoryPool was closed, the memory segments could be disposed without closing the gate in the NativeMemoryCleanerDrop.
The gate is important because it prevents double-frees of the memory segment.

Modification:
The fix is to change how the SizeClassedMemoryPool is closed, such that it always releases memory by calling `close()` on its buffers, which in turn will close the gate. The program will then proceed through the SizeClassedMemoryPool.drop implementation, which in turn will observe that the allocator is closed, and *then* dispose of the memory.

Result:
We should hopefully not see any more random test failures, but if we do, they would at least indicate a different bug.
This particular one was mostly showing up inside the cleaner threads, which were ignoring the exception, but occasionally I think the race went the other way, causing a test failure.
2020-12-07 17:35:21 +01:00
Chris Vest
2f99ee64a4
Merge pull request #14 from netty/benchmark-send
Add a benchmark for Buf.send()
2020-12-04 21:18:46 +01:00
Chris Vest
0c40143f5f Fix license header years, and style updates 2020-12-04 18:48:06 +01:00
Chris Vest
80185abec4 Add a benchmark for Buf.send()
Motivation:
This will likely be a somewhat common operation, as buffers move between eventloop and worker threads, so it's important to have an understanding of how it performs.

Modification:
Add a benchmark that specifically targets the send() operation on buffers.

Result:
We got benchmark numbers that clearly show the cost of confinement transfer
2020-12-04 16:27:08 +01:00
Chris Vest
236097e081 Make it possible to extend composite buffers after creation
Motivation:
Composite buffers are uniquely positioned to be able to extend their underlying storage relatively cheaply.
This fact is relied upon in a couple of buffer use cases within Netty, that we wish to support.

Modification:
Add a static `extend` method to Allocator, so that the CompositeBuf class can remain internal.
The `extend` method inserts the extension buffer at the end of the composite buffer as if it had been included from the start.
This involves checking offsets and byte order invariants.
We also require that the composite buffer be in an owned state.

Result:
It's now possible to extend a composite buffer with a specific buffer, after the composite buffer has been created.
2020-12-03 17:48:28 +01:00
Chris Vest
b0da25d888
Merge pull request #10 from netty/byte-itr-benchmark
Add benchmarks for ByteIterator
2020-12-02 15:14:18 +01:00
Chris Vest
6b7ea5f5cb Add benchmarks for ByteIterator
Motivation:
Capture the performance characteristics of this primitive for various buffer implementations.

Modification:
Add a benchmark that iterate 4KiB buffers forwards, and backwards, on various buffer implementations.

Result:
Another aspect of the implementation covered by benchmarks.
Turns out the composite iterators a somewhat slow.
2020-12-02 14:54:02 +01:00
Chris Vest
fcd97af4f9
Merge pull request #7 from netty/over-eager-cleaner
@chrisvest Capture build artifacts for failed builds
2020-12-01 14:58:06 +01:00
Chris Vest
e3c7f9b632 Capture build artifacts for failed builds
Motivation:
When a build fails, it's desirable to have the build artifacts available
so the build failure can be inspected, investigated and hopefully fixed.

Modification:
Change the Makefile and CI workflow such that the build artifacts are
captured and uploaded when the build fails.

Result:
A "target.zip" file is now available for download on failed GitHub
Actions builds.
2020-12-01 14:38:09 +01:00
Chris Vest
e039f6f7f5
Merge pull request #9 from netty/more-close-benchmarks
Add benchmark for closing pooled buffers
2020-12-01 11:46:42 +01:00
Chris Vest
4a409d2458 Add benchmark for closing pooled buffers
Motivation:
Pooled buffers are a very important use case, and they change the cost dynamics around shared memory segments, so it's worth looking into in detail.

Modification:
Add another explicit close of pooled direct buffers to MemorySegmentClosedByCleanerBenchmark

Result:
Explicitly closing of pooled buffers is even out-performing cleaner close on the "heavy" workload, so this is currently the fastest way to run that workload:

Benchmark                                                  (workload)  Mode  Cnt   Score   Error  Units
MemorySegmentClosedByCleanerBenchmark.cleanerClose              heavy  avgt  150  14,194 ± 0,558  us/op
MemorySegmentClosedByCleanerBenchmark.explicitClose             heavy  avgt  150  40,496 ± 0,414  us/op
MemorySegmentClosedByCleanerBenchmark.explicitPooledClose       heavy  avgt  150  12,723 ± 0,134  us/op
2020-12-01 11:15:44 +01:00
Chris Vest
89860b779a
Merge pull request #5 from netty/faster-send
Make Buf.send() faster
2020-11-26 13:59:47 +01:00
Chris Vest
a3f6ae6be8 Make Buf.send() faster
When send() a confined buffer, we had to first turn it into a shard buffer, so that it could be claimed by an arbitrary recipient thread.

As we've learned, however, closing shared segments is expensive.
We can speed up the send() call by simply leaving the segment shared.
This weakens the confinement of the received segment, though.
Currently no tests fails on that, but in the future we should re-implement confinement checking inside the Buf implementations themselves any, because pooled buffers also violate the confinement restriction, and we have a guiding principle that all buffers, regardless of implementation, should always behave the same.

The results of this change can be observed in the MemorySegmentClosedByCleanerBenchmark, with the heavy workload.
Explicitly closed segments now run the workload twice as fast, and the cleaner based closing is now 3 times faster.

Before:
Benchmark                                            (workload)  Mode  Cnt   Score   Error  Units
MemorySegmentClosedByCleanerBenchmark.cleanerClose        heavy  avgt  150  42,221 ± 0,943  us/op
MemorySegmentClosedByCleanerBenchmark.explicitClose       heavy  avgt  150  65,215 ± 0,761  us/op

After:
Benchmark                                            (workload)  Mode  Cnt   Score   Error  Units
MemorySegmentClosedByCleanerBenchmark.cleanerClose        heavy  avgt  150  13,871 ± 0,544  us/op
MemorySegmentClosedByCleanerBenchmark.explicitClose       heavy  avgt  150  37,516 ± 0,426  us/op
2020-11-26 11:31:23 +01:00
Chris Vest
34a58a763c
Merge pull request #4 from netty/benchmarks2
Add benchmarks examining the performance difference between explicitly closing buffers, and letting Cleaners close the buffers
2020-11-26 11:12:59 +01:00
Chris Vest
6364c4d170 Add a benchmark for examining the performance difference between explicitly closing memory segments, versus having them closed by cleaners 2020-11-26 10:37:14 +01:00
Chris Vest
f611d58a6e
Merge pull request #3 from netty/benchmarks
Add benchmarks for opening/closing shared/confined native/heap memory segments
2020-11-26 10:19:39 +01:00
Chris Vest
6078465721 Add a benchmark for opening and closing shared/confined native/heap memory segments 2020-11-24 10:56:22 +01:00
Chris Vest
cd9f84e856 The assertj-core dependency should only be available in test scope 2020-11-23 18:11:22 +01:00
Chris Vest
92c178ceb9 The BufTest.pooledBuffersMustResetStateBeforeReuse should run for all allocators 2020-11-23 18:10:58 +01:00