ptupitsyn commented on code in PR #4142:
URL: https://github.com/apache/ignite-3/pull/4142#discussion_r1711178442
##########
modules/binary-tuple/src/main/java/org/apache/ignite/internal/binarytuple/inlineschema/TupleWithSchemaMarshalling.java:
##########
@@ -38,17 +38,23 @@ public final class TupleWithSchemaMarshalling {
/**
* Marshal tuple is the following format:
*
- * <p>marshalledTuple := | size | binaryTupleWithSchema |
+ * <p>marshalledTuple := | size | valuePosition |
binaryTupleWithSchema |
*
* <p>size := int32
*
- * <p>binaryTupleWithSchema := | column1 | ... | columnN | value1 | ... |
valueN |
+ * <p>valuePosition := int32
Review Comment:
Let's specify endianness.
##########
modules/binary-tuple/src/main/java/org/apache/ignite/internal/binarytuple/inlineschema/TupleWithSchemaMarshalling.java:
##########
@@ -65,64 +71,105 @@ public final class TupleWithSchemaMarshalling {
types[i] = inferType(value);
}
- BinaryTupleBuilder builder = builder(values, columns, types);
+ BinaryTupleBuilder schemaBuilder = schemaBuilder(columns, types);
+ BinaryTupleBuilder valueBuilder = valueBuilder(values, types);
+
+ byte[] schemaBt = schemaBuilder.build().array();
+ byte[] valueBt = valueBuilder.build().array();
+ // Size: int32 (tuple size), int32 (value offset), schema, value.
+ byte[] result = new byte[4 + 4 + schemaBt.length + valueBt.length];
- byte[] arr = builder.build().array();
- byte[] result = new byte[arr.length + 1];
- // Put the size of the schema in the first byte.
- result[0] = (byte) size; // todo should be int32
+ // Put the size of the schema in the first 4 bytes.
+ result[0] = (byte) (size >> 24);
+ result[1] = (byte) (size >> 16);
+ result[2] = (byte) (size >> 8);
+ result[3] = (byte) size;
- System.arraycopy(arr, 0, result, 1, arr.length);
+ // Put the value offset in the second 4 bytes.
+ int offset = schemaBt.length + 8;
+ result[4] = (byte) (offset >> 24);
+ result[5] = (byte) (offset >> 16);
+ result[6] = (byte) (offset >> 8);
+ result[7] = (byte) offset;
+
+ System.arraycopy(schemaBt, 0, result, 8, schemaBt.length);
+ System.arraycopy(valueBt, 0, result, schemaBt.length + 8,
valueBt.length);
return result;
}
+
/**
* Unmarshal tuple.
*
* @param raw byte[] bytes that are marshaled by {@link #marshal(Tuple)}.
*/
public static @Nullable Tuple unmarshal(byte @Nullable [] raw) {
- byte size = raw[0]; // todo should be int32
+ // Read first int32.
+ int size = ((raw[0] & 0xFF) << 24) |
+ ((raw[1] & 0xFF) << 16) |
+ ((raw[2] & 0xFF) << 8) |
+ (raw[3] & 0xFF);
+
+ // Read second int32.
+ int valueOffset = ((raw[4] & 0xFF) << 24) |
+ ((raw[5] & 0xFF) << 16) |
+ ((raw[6] & 0xFF) << 8) |
+ (raw[7] & 0xFF);
Review Comment:
Use `ByteBuffer` methods - we need it below anyway.
##########
modules/binary-tuple/src/main/java/org/apache/ignite/internal/binarytuple/inlineschema/TupleWithSchemaMarshalling.java:
##########
@@ -65,64 +71,105 @@ public final class TupleWithSchemaMarshalling {
types[i] = inferType(value);
}
- BinaryTupleBuilder builder = builder(values, columns, types);
+ BinaryTupleBuilder schemaBuilder = schemaBuilder(columns, types);
+ BinaryTupleBuilder valueBuilder = valueBuilder(values, types);
+
+ byte[] schemaBt = schemaBuilder.build().array();
+ byte[] valueBt = valueBuilder.build().array();
+ // Size: int32 (tuple size), int32 (value offset), schema, value.
+ byte[] result = new byte[4 + 4 + schemaBt.length + valueBt.length];
- byte[] arr = builder.build().array();
- byte[] result = new byte[arr.length + 1];
- // Put the size of the schema in the first byte.
- result[0] = (byte) size; // todo should be int32
+ // Put the size of the schema in the first 4 bytes.
+ result[0] = (byte) (size >> 24);
+ result[1] = (byte) (size >> 16);
+ result[2] = (byte) (size >> 8);
+ result[3] = (byte) size;
- System.arraycopy(arr, 0, result, 1, arr.length);
+ // Put the value offset in the second 4 bytes.
+ int offset = schemaBt.length + 8;
+ result[4] = (byte) (offset >> 24);
+ result[5] = (byte) (offset >> 16);
+ result[6] = (byte) (offset >> 8);
+ result[7] = (byte) offset;
+
+ System.arraycopy(schemaBt, 0, result, 8, schemaBt.length);
+ System.arraycopy(valueBt, 0, result, schemaBt.length + 8,
valueBt.length);
return result;
}
+
/**
* Unmarshal tuple.
*
* @param raw byte[] bytes that are marshaled by {@link #marshal(Tuple)}.
*/
public static @Nullable Tuple unmarshal(byte @Nullable [] raw) {
- byte size = raw[0]; // todo should be int32
+ // Read first int32.
+ int size = ((raw[0] & 0xFF) << 24) |
+ ((raw[1] & 0xFF) << 16) |
+ ((raw[2] & 0xFF) << 8) |
+ (raw[3] & 0xFF);
+
+ // Read second int32.
+ int valueOffset = ((raw[4] & 0xFF) << 24) |
+ ((raw[5] & 0xFF) << 16) |
+ ((raw[6] & 0xFF) << 8) |
+ (raw[7] & 0xFF);
String[] columns = new String[size];
ColumnType[] types = new ColumnType[size];
- byte[] arr = new byte[raw.length - 1];
- System.arraycopy(raw, 1, arr, 0, arr.length);
- BinaryTupleReader reader = new BinaryTupleReader(size * 3, arr);
+ // Valueoffset (*) points at the index where the valueBt starts,
+ // the length of the destination schemaArr is calculated as
+ // | int32 | int32 | schema bytes |(*) value bytes |
+ // (*) - int32 - int32 = len(schema bytes)
+ byte[] schemaArr = new byte[valueOffset - 4 - 4];
+ byte[] valueArr = new byte[raw.length - valueOffset];
+
+ System.arraycopy(raw, 8, schemaArr, 0, schemaArr.length);
+ System.arraycopy(raw, valueOffset, valueArr, 0, valueArr.length);
Review Comment:
Unnecessary copies. Let's use a `ByteBuffer` instead - another
`BinaryTupleReader` constructor accepts it.
##########
modules/binary-tuple/src/main/java/org/apache/ignite/internal/binarytuple/inlineschema/TupleWithSchemaMarshalling.java:
##########
@@ -65,64 +71,105 @@ public final class TupleWithSchemaMarshalling {
types[i] = inferType(value);
}
- BinaryTupleBuilder builder = builder(values, columns, types);
+ BinaryTupleBuilder schemaBuilder = schemaBuilder(columns, types);
+ BinaryTupleBuilder valueBuilder = valueBuilder(values, types);
+
+ byte[] schemaBt = schemaBuilder.build().array();
+ byte[] valueBt = valueBuilder.build().array();
+ // Size: int32 (tuple size), int32 (value offset), schema, value.
+ byte[] result = new byte[4 + 4 + schemaBt.length + valueBt.length];
- byte[] arr = builder.build().array();
- byte[] result = new byte[arr.length + 1];
- // Put the size of the schema in the first byte.
- result[0] = (byte) size; // todo should be int32
+ // Put the size of the schema in the first 4 bytes.
+ result[0] = (byte) (size >> 24);
Review Comment:
Let's extract this logic to an utility class. Note that we have
`HeapByteBufUtil` in raft module, might want to reuse it.
##########
modules/binary-tuple/src/main/java/org/apache/ignite/internal/binarytuple/inlineschema/TupleWithSchemaMarshalling.java:
##########
@@ -65,64 +71,105 @@ public final class TupleWithSchemaMarshalling {
types[i] = inferType(value);
}
- BinaryTupleBuilder builder = builder(values, columns, types);
+ BinaryTupleBuilder schemaBuilder = schemaBuilder(columns, types);
+ BinaryTupleBuilder valueBuilder = valueBuilder(values, types);
+
+ byte[] schemaBt = schemaBuilder.build().array();
+ byte[] valueBt = valueBuilder.build().array();
+ // Size: int32 (tuple size), int32 (value offset), schema, value.
+ byte[] result = new byte[4 + 4 + schemaBt.length + valueBt.length];
- byte[] arr = builder.build().array();
- byte[] result = new byte[arr.length + 1];
- // Put the size of the schema in the first byte.
- result[0] = (byte) size; // todo should be int32
+ // Put the size of the schema in the first 4 bytes.
+ result[0] = (byte) (size >> 24);
Review Comment:
Alternatively, use `ByteBuffer`.
--
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]
