stoty commented on code in PR #5943:
URL: https://github.com/apache/hbase/pull/5943#discussion_r1615540093
##########
hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/ProtobufMessageHandler.java:
##########
@@ -18,21 +18,55 @@
package org.apache.hadoop.hbase.rest;
import java.io.IOException;
+import java.io.OutputStream;
import org.apache.yetus.audience.InterfaceAudience;
+import org.apache.hbase.thirdparty.com.google.protobuf.CodedOutputStream;
+import org.apache.hbase.thirdparty.com.google.protobuf.Message;
+
/**
* Common interface for models capable of supporting protobuf marshalling and
unmarshalling. Hooks
* up to the ProtobufMessageBodyConsumer and ProtobufMessageBodyProducer
adapters.
*/
@InterfaceAudience.Private
public interface ProtobufMessageHandler {
- /** Returns the protobuf represention of the model */
- byte[] createProtobufOutput();
+
+ // The Jetty 9.4 HttpOutput default commit size is 32K/4 = 8K. We use that
size to avoid
+ // double buffering (and copying) in HttpOutput. If we ever increase the
HttpOutput commit size,
+ // we need to adjust this accordingly. We should also revisit this when
Jetty is upgraded.
+ static final int BUFFER_SIZE = 8 * 1024;
Review Comment:
Thank you, fixed.
##########
hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/ProtobufMessageHandler.java:
##########
@@ -18,21 +18,55 @@
package org.apache.hadoop.hbase.rest;
import java.io.IOException;
+import java.io.OutputStream;
import org.apache.yetus.audience.InterfaceAudience;
+import org.apache.hbase.thirdparty.com.google.protobuf.CodedOutputStream;
+import org.apache.hbase.thirdparty.com.google.protobuf.Message;
+
/**
* Common interface for models capable of supporting protobuf marshalling and
unmarshalling. Hooks
* up to the ProtobufMessageBodyConsumer and ProtobufMessageBodyProducer
adapters.
*/
@InterfaceAudience.Private
public interface ProtobufMessageHandler {
- /** Returns the protobuf represention of the model */
- byte[] createProtobufOutput();
+
+ // The Jetty 9.4 HttpOutput default commit size is 32K/4 = 8K. We use that
size to avoid
+ // double buffering (and copying) in HttpOutput. If we ever increase the
HttpOutput commit size,
+ // we need to adjust this accordingly. We should also revisit this when
Jetty is upgraded.
+ static final int BUFFER_SIZE = 8 * 1024;
+
+ /** Writes the protobuf represention of the model */
+ default void writeProtobufOutput(OutputStream os) throws IOException {
+ // Creating an explicit CodedOutputStream for the following reasons :
+ // 1. This avoids the cost of pre-computing the message size
+ // 2. This lets us set the buffer size explicitly
+ CodedOutputStream cos = CodedOutputStream.newInstance(os, BUFFER_SIZE);
+ messageFromObject().writeTo(cos);
+ cos.flush();
+ }
+
+ /**
+ * Use {@link
org.apache.hadoop.hbase.rest#writeProtobufOutput(OutputStream)} with
+ * BufferedOutputStream for better performance
+ * @return the protobuf encoded object in a byte array
+ */
+ default byte[] createProtobufOutput() {
+ return messageFromObject().toByteArray();
+ }
+
+ public Message messageFromObject();
Review Comment:
Thank you, fixed.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
