apache / apache/bookkeeper

DirectMemoryCRC32Digest.update(ByteBuf) allocates lots of byte[]

Open
#2,064 6 comments 0 reactions 0 assignees View on GitHub
area/client bk-client-perf triage/week-33 type/feature
Dominant language
Java
Stars
2k
Forks
976
Avg merge
6d 15h
Merged PRs (30d)
7

Description

**FEATURE REQUEST**

1. Please describe the feature you are requesting.
Profiling shows the code block in bold is hit every time an entry is read from bookie.

```
@Override
public void update(ByteBuf buf) {
int index = buf.readerIndex();
int length = buf.readableBytes();

try {
if (buf.hasMemoryAddress()) {
// Calculate CRC directly from the direct memory pointer
crcValue = (int) updateByteBuffer.invoke(null, crcValue, buf.memoryAddress(), index, length);
} else if (buf.hasArray()) {
// Use the internal method to update from array based
crcValue = (int) updateBytes.invoke(null, crcValue, buf.array(), buf.arrayOffset() + index, length);
} else {
**// Fallback to data copy if buffer is not contiguous
byte[] b = new byte[length];
buf.getBytes(index, b, 0, length);
crcValue = (int) updateBytes.invoke(null, crcValue, b, 0, b.length);**
}
} catch (IllegalAccessException | InvocationTargetException e) {
throw new RuntimeException(e);
}
}
```
This is due to ByteBuf being marked as read only (as a result buf.hasArray() == false) from
```
ReadCompletion
public void handleV3Response(BookkeeperProtocol.Response response) {
readEntryOutstanding.dec();
ReadResponse readResponse = response.getReadResponse();
StatusCode status = response.getStatus() == StatusCode.EOK
? readResponse.getStatus() : response.getStatus();
ByteBuf buffer = Unpooled.EMPTY_BUFFER;
if (readResponse.hasBody()) {
**buffer = Unpooled.wrappedBuffer(readResponse.getBody().asReadOnlyByteBuffer());**
}
```

2. Indicate the importance of this issue to you (blocker, must-have, should-have, nice-to-have).
Are you currently using any workarounds to address this issue?

This generates lots of garbage. See attached image. Would be nice to get your thoughts on this issue, maybe using a buffer pool.
![bk_digest_memory](https://user-images.githubusercontent.com/49574764/56166464-8ac80280-5f8a-11e9-9abb-b8baf805a641.png)

3. Provide any additional detail on your proposed use case for this feature.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.