pnowojski commented on a change in pull request #10529: [FLINK-15171]
[serialization] fix performance regression caused by too many buffer
allocations on string serialization
URL: https://github.com/apache/flink/pull/10529#discussion_r357918705
##########
File path: flink-core/src/main/java/org/apache/flink/types/StringValue.java
##########
@@ -776,9 +784,16 @@ public static String readString(DataInput in) throws
IOException {
* to accommodate for the next characters.
*/
- // happily assume that the string is an 7 bit us-ascii one
- byte[] buf = new byte[len];
- in.readFully(buf);
+ byte[] buf;
+ if (len < SHORT_STRING_MAX_LENGTH) {
+ // skip allocating a separate buffer and reuse the
thread-local one.
+ // as allocating the buffer for small strings can
produce too much GC pressure.
+ buf = shortStringBuffer.get();
+ in.readFully(buf, 0, len);
+ } else {
+ buf = new byte[len];
+ in.readFully(buf);
+ }
Review comment:
Hey. Definitely one issue with this approach is that we do not have
benchmark coverage for strings > 1KB.
`org.apache.flink.benchmark.SerializationFrameworkMiniBenchmarks#serializerHeavyString`
as I can see is using just 1KB strings? Maybe it just barely qualifies for the
non `ThreadLocal` branch, but if we wanted to go such direction, we would need
to either decrease the size of the buffer (100?) or incease strings in
`serializerHeavyString` (`SHORT_STRING_MAX_LENGTH * 1.5` ?).
However do we need such differentiation? Couldn't this have this one single
per thread fixed size buffer, but if the string is larger the buffer max size,
read/write string from/to the fixed size buffer size in steps? like:
```
for step in 0..string.size()/buf.length:
for i in 0..buf.length:
buf[i] = string[step * buf.length + i] // or in.readFully?
// process buffer
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
