Make ES query recording allocation free #283
Conversation
| public class Db implements Recyclable { | ||
|
|
||
| private final ObjectPool<CharBuffer> charBufferPool = new QueueBasedObjectPool<CharBuffer>(new MpmcAtomicArrayQueue<>(128), false, | ||
| new Allocator<CharBuffer>() { |
There was a problem hiding this comment.
stay consistent with the other pool initialization, not through usage of new
There was a problem hiding this comment.
You mean I should add a static initializer for creating a QueueBasedObjectPool with an Allocator and Resetter? Or do you mean I should not use an anonymous inner class for Allocator?
There was a problem hiding this comment.
created QueueBasedObjectPool.of static initializer
There was a problem hiding this comment.
I was referring to the ObjectPool- I saw you changed all others to use a static initializer, so I wondered why not this one as well.
Anyway, not very important
| return this; | ||
| } | ||
|
|
||
| public CharBuffer withStatementBuffer() { |
There was a problem hiding this comment.
Add documentation that user of this method is not allowed to hold a reference to the buffer
| } | ||
|
|
||
| @Nullable | ||
| public CharBuffer getStatementBuffer() { |
apm-agent-core/src/main/java/co/elastic/apm/impl/transaction/Db.java
Outdated
Show resolved
Hide resolved
| final CharsetDecoder charsetDecoder = threadLocalCharsetDecoder.get(); | ||
| try (is) { | ||
| final byte[] bufferArray = buffer.array(); | ||
| for (int read = is.read(bufferArray); read != -1; read = is.read(bufferArray)) { |
There was a problem hiding this comment.
Start by checking whether this is UTF-8 encoded indeed (by checking whether first byte is {)
There was a problem hiding this comment.
This is handled neatly by the coder result return value of charsetDecoder.decode. In case a character could not be mapped, coderResult.isError() returns true. However, I should clear() the CharBuffer in that case, so that it does not get serialized.
There was a problem hiding this comment.
If encoding was done using UTF-16, you won't necessarily get an error decoding with UTF-8. You may just get the wrong decoding.
But right about the clearing buffer whenever decoding fails!
There was a problem hiding this comment.
I have a test exactly for this which asserts that the encoding fails if the steam is encoded in UTF-16: https://github.com/elastic/apm-agent-java/pull/283/files#diff-97c7f94f59c329dd827193f8a343fc90R82
There was a problem hiding this comment.
Ahh, since the first char is always {, a UTF-16 will produce a first 0x00 byte...
|
+1 on removing the old code |
Codecov Report
@@ Coverage Diff @@
## master #283 +/- ##
============================================
- Coverage 73.29% 73.23% -0.07%
- Complexity 1158 1165 +7
============================================
Files 124 126 +2
Lines 4322 4372 +50
Branches 431 437 +6
============================================
+ Hits 3168 3202 +34
- Misses 947 961 +14
- Partials 207 209 +2
Continue to review full report at Codecov.
|
|
|
||
| @VisibleForAdvice | ||
| public class ESRestClientInstrumentationHelper { | ||
| public class IOUtils { |
3 participants
No description provided.