Add direct access to current recycler stream page #135114
Conversation
… recycler_byte_improvements
… recycler_byte_improvements
… recycler_byte_improvements
… recycler_byte_improvements
…vements_with_bench
Tim-Brooks
added
>non-issue
:Distributed Indexing/CRUD
|
Pinging @elastic/es-distributed-indexing (Team:Distributed Indexing) |
elasticsearchmachine
added
the
Team:Distributed Indexing
…vements_with_bench
…vements_with_bench
| @OutputTimeUnit(TimeUnit.NANOSECONDS) | ||
| @State(Scope.Thread) | ||
| @Fork(value = 1) | ||
| public class RecyclerBytesStreamBenchmark { |
There was a problem hiding this comment.
Added a microbenchmark to allow testing the methods I was working on.
| this.currentPageOffset = pageSize; | ||
| // Always start with a page | ||
| ensureCapacityFromPosition(1); | ||
| nextPage(); |
There was a problem hiding this comment.
Always start with an initial page to remove the requirement that the page allocation is in one of the write methods otherwise.
| if (1 > (pageSize - currentPageOffset)) { | ||
| if (1 > pageSize - currentPageOffset) { | ||
| ensureCapacity(1); | ||
| nextPage(); |
There was a problem hiding this comment.
Change because ensureCapacity now JUST allocates sufficient pages. Does not adjust write location.
| ensureCapacity(length); | ||
| currentPageOffset = this.currentPageOffset; | ||
| writeMultiplePages(b, offset, length); | ||
| } else { |
There was a problem hiding this comment.
Specifically add a branch for the extremely common scenario where the bytes fit in the current page.
| } | ||
|
|
||
| @Override | ||
| public void writeVInt(int i) throws IOException { |
There was a problem hiding this comment.
vInt is very commonly written (header for bytes reference, map item count, etc, etc). Since it is pretty expensive to write we create a specific variant that works for the recycler stream variant.
| if (offsetInPage == 0) { | ||
| this.pageIndex = pageIndex - 1; | ||
| this.currentPageOffset = pageSize; | ||
| if (position > 0) { |
There was a problem hiding this comment.
This method is the same except we need special handling for position 0 since the stream always has at least one page.
| out.writeByte(b); | ||
| assertEquals(b, out.bytes().get(PageCacheRecycler.BYTE_PAGE_SIZE)); | ||
| } | ||
|
|
There was a problem hiding this comment.
More test coverage.
This change makes a number of minor optimizations to the recycler bytes
stream. The most important change is that it allow cached direct access
to the current page. This helps in most scenarios where are write does
not cross page boundaries.
Additionally, it enables future subclasses to implement custom
serialization directly to the page with minimal bounds checks.
Finally, it always creates the first page in the ctor to remove guaranteed
expand calls in the first stream write.