RussellSpitzer commented on code in PR #13284: URL: https://github.com/apache/iceberg/pull/13284#discussion_r2138553860
########## core/src/main/java/org/apache/iceberg/variants/PrimitiveWrapper.java: ########## @@ -111,10 +113,13 @@ public int sizeInBytes() { case BINARY: return 5 + ((ByteBuffer) value).remaining(); // 1 header + 4 length + value length case STRING: + byte[] valueBytes = ((String) value).getBytes(StandardCharsets.UTF_8); Review Comment: I think we are losing one of the optimizations we previously had here by always calling getBytes on value even if the buffer is already set. I haven't looked into the details here but I think we can do something like ```java case STRING: if (null == buffer) { this.buffer = ByteBuffer.wrap(((String) value).getBytes(StandardCharsets.UTF_8)); } if (buffer.remaining() < MAX_SHORT_STRING_LENGTH) { return 1 + buffer.remaining() // Short String : 1 header + value length } else { return 5 + buffer.remaining(); // String Primitive: 1 header + 4 length + value length } ``` -- 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. To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org