fishy commented on a change in pull request #2439:
URL: https://github.com/apache/thrift/pull/2439#discussion_r725557980



##########
File path: lib/java/src/org/apache/thrift/protocol/TProtocol.java
##########
@@ -180,4 +181,145 @@ public void reset() {}
   public Class<? extends IScheme> getScheme() {
     return StandardScheme.class;
   }
+
+  // -----------------------------------------------------------------
+  // Additional methods to improve performance.
+
+  public int readFieldBeginData() throws TException {
+    // Derived classes should provide a more efficient version of this
+    // method if allowed by the encoding used by that protocol.
+    TField tfield = this.readFieldBegin();
+    return TFieldData.encode(tfield.type, tfield.id);
+  }
+
+  public void skip(byte fieldType) throws TException {
+    this.skip(fieldType, Integer.MAX_VALUE);
+  }
+
+  public void skip(byte fieldType, int maxDepth) throws TException {
+    if (maxDepth <= 0) {
+      throw new TException("Maximum skip depth exceeded");
+    }
+
+    switch (fieldType) {
+      case TType.BOOL:
+        this.skipBool();
+        break;
+
+      case TType.BYTE:
+        this.skipByte();
+        break;
+
+      case TType.I16:
+        this.skipI16();
+        break;
+
+      case TType.I32:
+        this.skipI32();
+        break;
+
+      case TType.I64:
+        this.skipI64();
+        break;
+
+      case TType.DOUBLE:
+        this.skipDouble();
+        break;
+
+      case TType.STRING:
+        this.skipBinary();
+        break;
+
+      case TType.STRUCT:
+        this.readStructBegin();
+        while (true) {
+          int tfieldData = this.readFieldBeginData();
+          byte tfieldType = TFieldData.getType(tfieldData);
+          if (tfieldType == TType.STOP) {
+            break;
+          }
+          this.skip(tfieldType, maxDepth - 1);
+          this.readFieldEnd();
+        }
+        this.readStructEnd();
+        break;
+
+      case TType.MAP:
+        TMap map = this.readMapBegin();
+        for (int i = 0; i < map.size; i++) {
+          this.skip(map.keyType, maxDepth - 1);
+          this.skip(map.valueType, maxDepth - 1);
+        }
+        this.readMapEnd();
+        break;
+
+      case TType.SET:
+        TSet set = this.readSetBegin();
+        for (int i = 0; i < set.size; i++) {
+          this.skip(set.elemType, maxDepth - 1);
+        }
+        this.readSetEnd();
+        break;
+
+      case TType.LIST:
+        TList list = this.readListBegin();
+        for (int i = 0; i < list.size; i++) {
+          this.skip(list.elemType, maxDepth - 1);
+        }
+        this.readListEnd();
+        break;
+
+      default:
+        throw new TProtocolException(
+            TProtocolException.INVALID_DATA, "Unrecognized type " + fieldType);
+    }
+  }
+
+  protected void skipBool() throws TException {
+    this.skipBytes(1);
+  }
+
+  protected void skipByte() throws TException {
+    this.skipBytes(1);
+  }
+
+  protected void skipI16() throws TException {
+    this.skipBytes(2);
+  }
+
+  protected void skipI32() throws TException {
+    this.skipBytes(4);
+  }
+
+  protected void skipI64() throws TException {
+    this.skipBytes(8);
+  }
+
+  protected void skipDouble() throws TException {
+    this.skipBytes(8);
+  }

Review comment:
       That is fine, but then you need to fix all the `TProtocol` 
implementations that use different bytes here, which includes `TJSONProtocol` 
and `TSimpleJSONProtocol`, and you did not "fix" them.
   
   `TProtocol` is not some thing "either binary or compact". there are a lot 
more of them, and the fixed numbers are only correct for `TBInaryProtocol`. So 
it really should be the other way around.




-- 
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]


Reply via email to