Make BytesStreamOutput more efficient #5318
Conversation
via a (currently private) non-recycling PageCacheRecycler. This change brought to light a bug in FsTranslog, which used seek() incorrectly/ambiguously; this is fixed so that it works with both the old and new implementation. Fix for bug #5159
| // acquire all other requested pages up front if expectedSize > pageSize | ||
| if (expectedSize > pageSize) { | ||
| ensureCapacity(expectedSize); | ||
| } |
There was a problem hiding this comment.
I'm wondering if there is a benefit in pre-allocating those pages compared to requesting them when necessary?
There was a problem hiding this comment.
We are only preallocating (pre-requesting) when the initial capacity is given by the client. By default we only acquire a single page to get started. Strictly speaking it's not necessary, but clients who announce a capacity goal are expected to use it. Also, once we actually start using a real recycler, this will reduce contention (all pages for one acquire).
|
Part of the code looks very similar to what we have in |
| byte[] page = pages.get(count / pageSize).v(); | ||
| int offset = count % pageSize; | ||
| page[offset] = b; | ||
| count++; | ||
| } | ||
|
|
||
| @Override | ||
| public void writeBytes(byte[] b, int offset, int length) throws IOException { |
There was a problem hiding this comment.
When doing many small writes, it may help to keep a reference on the current page to not have to recompute it every time?
There was a problem hiding this comment.
That's a good point - for now I was more concerned about getting it right every time, but adding a currentPage pointer should not be too difficult. Since we will likely revisit this code soon I'd like to keep this as it is for now.
|
I just discussed with @kimchy about this change and it might make sense to use |
A new metric aggregation that can compute approximate values of arbitrary percentiles. Close #5323
This commit fixes ensures that for external builds (e.g. plugin development) that the REST tests that are copied are properly filtered to only include the API by default. The code prior to this change resulted in including both the API and tests since the copy.include resulted as an empty list by default since the stream is empty unless explicitly configured. related #52114 fixes #53183
This commit fixes ensures that for external builds (e.g. plugin development) that the REST tests that are copied are properly filtered to only include the API by default. The code prior to this change resulted in including both the API and tests since the copy.include resulted as an empty list by default since the stream is empty unless explicitly configured. related #52114 fixes #53183
Makes BytesStreamOutput more efficient by introducing internal paging via a (currently private) non-recycling PageCacheRecycler. This change brought to light a bug in FsTranslog, which used seek() incorrectly/ambiguously; this is fixed so that it works with both the old and new implementation.
Fix for bug #5159