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:
Definitely one issue with this approach is that we do not have benchmark
coverage for strings > 1KB.
`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 even need such `if (len < SHORT_STRING_MAX_LENGTH)` branch
(both here and below)? Couldn't this have a single per thread fixed size
buffer, but if the string is larger then the buffer's 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
