ascherbakoff commented on a change in pull request #168: URL: https://github.com/apache/ignite-3/pull/168#discussion_r655224609
########## File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/ChunkFormat.java ########## @@ -0,0 +1,299 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.schema.row; + +import org.apache.ignite.internal.schema.BinaryRow; + +/** + * Chunk format. + */ +abstract class ChunkFormat { + /** First two flag bits reserved for format code. */ + public static final int FORMAT_CODE_MASK = 0x03; + + /** Flag indicates key chunk omits vartable. */ + public static final int OMIT_NULL_MAP_FLAG = 1 << 2; + + /** Flag indicates value chunk omits null map. */ + public static final int OMIT_VARTBL_FLAG = 1 << 3; + + /** Writer factory for tiny-sized chunks. */ + private static final ChunkFormat TINY = new ChunkFormat(Byte.BYTES, Byte.BYTES, (byte)1) { Review comment: Nested class declarations should be put to the end of class file ########## File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/ChunkFormat.java ########## @@ -0,0 +1,299 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.schema.row; + +import org.apache.ignite.internal.schema.BinaryRow; + +/** + * Chunk format. + */ +abstract class ChunkFormat { + /** First two flag bits reserved for format code. */ + public static final int FORMAT_CODE_MASK = 0x03; + + /** Flag indicates key chunk omits vartable. */ + public static final int OMIT_NULL_MAP_FLAG = 1 << 2; + + /** Flag indicates value chunk omits null map. */ + public static final int OMIT_VARTBL_FLAG = 1 << 3; + + /** Writer factory for tiny-sized chunks. */ + private static final ChunkFormat TINY = new ChunkFormat(Byte.BYTES, Byte.BYTES, (byte)1) { + /** {@inheritDoc} */ + @Override void writeVarlenOffset(ExpandableByteBuf buf, int vartblOff, int entryIdx, int off) { + assert off < (1 << 8) && off >= 0 : "Varlen offset overflow: offset=" + off; + + buf.put(vartblOff + vartableEntryOffset(entryIdx), (byte)off); + } + + /** {@inheritDoc} */ + @Override int readVarlenOffset(BinaryRow row, int vartblOff, int entryIdx) { + return row.readByte(vartblOff + vartableEntryOffset(entryIdx)) & 0xFF; + } + + /** {@inheritDoc} */ + @Override void writeVartableSize(ExpandableByteBuf buf, int vartblOff, int size) { + assert size < (1 << 8) && size >= 0 : "Vartable size overflow: size=" + size; + + buf.put(vartblOff, (byte)size); + } + + /** {@inheritDoc} */ + @Override int readVartableSize(BinaryRow row, int vartblOff) { + return row.readByte(vartblOff) & 0xFF; + } + }; + + /** Writer factory for med-sized chunks. */ + private static final ChunkFormat MEDIUM = new ChunkFormat(Short.BYTES, Short.BYTES, (byte)2) { + /** {@inheritDoc} */ + @Override void writeVarlenOffset(ExpandableByteBuf buf, int vartblOff, int entryIdx, int off) { + assert off < (1 << 16) && off >= 0 : "Varlen offset overflow: offset=" + off; + + buf.putShort(vartblOff + vartableEntryOffset(entryIdx), (short)off); + } + + /** {@inheritDoc} */ + @Override int readVarlenOffset(BinaryRow row, int vartblOff, int entryIdx) { + return row.readShort(vartblOff + vartableEntryOffset(entryIdx)) & 0xFFFF; + } + + /** {@inheritDoc} */ + @Override void writeVartableSize(ExpandableByteBuf buf, int vartblOff, int size) { + assert size < (1 << 16) && size >= 0 : "Vartable size overflow: size=" + size; + + buf.putShort(vartblOff, (short)size); + } + + /** {@inheritDoc} */ + @Override int readVartableSize(BinaryRow row, int vartblOff) { + return row.readShort(vartblOff) & 0xFFFF; + } + }; + + /** Writer factory for large-sized chunks. */ + private static final ChunkFormat LARGE = new ChunkFormat(Short.BYTES, Integer.BYTES, (byte)0) { + /** {@inheritDoc} */ + @Override void writeVarlenOffset(ExpandableByteBuf buf, int vartblOff, int entryIdx, int off) { + buf.putInt(vartblOff + vartableEntryOffset(entryIdx), off); + } + + /** {@inheritDoc} */ + @Override int readVarlenOffset(BinaryRow row, int vartblOff, int entryIdx) { + return row.readInteger(vartblOff + vartableEntryOffset(entryIdx)); + } + + /** {@inheritDoc} */ + @Override void writeVartableSize(ExpandableByteBuf buf, int vartblOff, int size) { + assert size < (1 << 16) && size >= 0 : "Vartable size overflow: size=" + size; + + buf.putShort(vartblOff, (short)size); + } + + /** {@inheritDoc} */ + @Override int readVartableSize(BinaryRow row, int vartblOff) { + return row.readShort(vartblOff) & 0xFFFF; + } + }; + + /** + * Return chunk formatter. + * + * @param payloadLen Payload size in bytes. + * @return Chunk formatter. + */ + static ChunkFormat formatter(int payloadLen) { Review comment: This approach can create different binary representations of a key, depending on the payloadLen. It should be avoided. For example, we can always use LARGE format for a key. ########## File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/Columns.java ########## @@ -58,6 +58,11 @@ */ private final int nullMapSize; + /** + * Estimated max length of fixed-size columns. + */ + private int fixsizeMaxLen; Review comment: ```suggestion private int fixedSizeMaxLen; ``` ########## File path: modules/schema/src/test/java/org/apache/ignite/internal/schema/RowAssemblerAdvancedSchemaTest.java ########## @@ -54,8 +55,8 @@ public void fixedNullableColumns() { // Last col null assertRowBytesEquals( - new byte[] {42, 0, 24, 0, -11, 43, 0, 0, 8, 0, 0, 0, 4, 11, 22, 0, 8, 0, 0, 0, 4, -44, -66, -1}, - new RowAssembler(schema, 0, 0, 0) + new byte[] {42, 0, 0, -103, -11, 43, 0, 0, 8, 0, 0, 0, 4, 11, 22, 0, 8, 0, 0, 0, 4, -44, -66, -1}, Review comment: Can we avoid comparing with byte array and instead compare expected values, like row0 -> serialize -> deserialize -> row1 assert row0 equals row1 ########## File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/RowAssembler.java ########## @@ -121,80 +106,101 @@ else if (Character.isHighSurrogate(ch)) { } /** - * @param keyCols Key columns. + * Creates RowAssembler for chunks of unknown size. + * <p> + * RowAssembler will use adaptive buffer size and omit some optimizations for small key/value chunks. + * + * @param schema Row schema. * @param nonNullVarlenKeyCols Number of non-null varlen columns in key chunk. - * @param nonNullVarlenKeySize Size of non-null varlen columns in key chunk. - * @param valCols Value columns. * @param nonNullVarlenValCols Number of non-null varlen columns in value chunk. - * @param nonNullVarlenValSize Size of non-null varlen columns in value chunk. - * @return Total row size. */ - public static int rowSize( - Columns keyCols, + public RowAssembler( + SchemaDescriptor schema, int nonNullVarlenKeyCols, - int nonNullVarlenKeySize, - Columns valCols, - int nonNullVarlenValCols, - int nonNullVarlenValSize + int nonNullVarlenValCols ) { - return BinaryRow.KEY_CHUNK_OFFSET /* Header size */ + - rowChunkSize(keyCols, nonNullVarlenKeyCols, nonNullVarlenKeySize) + - rowChunkSize(valCols, nonNullVarlenValCols, nonNullVarlenValSize); - } - - /** - * @param cols Columns. - * @param nonNullVarlenCols Number of non-null varlen columns in chunk. - * @param nonNullVarlenSize Size of non-null varlen columns in chunk. - * @return Row's chunk size. - */ - static int rowChunkSize(Columns cols, int nonNullVarlenCols, int nonNullVarlenSize) { - int size = BinaryRow.CHUNK_LEN_FIELD_SIZE + cols.nullMapSize() + - varlenTableChunkSize(nonNullVarlenCols); - - for (int i = 0; i < cols.numberOfFixsizeColumns(); i++) - size += cols.column(i).type().sizeInBytes(); - - return size + nonNullVarlenSize; + this(schema, + 0, + schema.keyColumns().nullMapSize() > 0, + nonNullVarlenKeyCols, + 0, + schema.valueColumns().nullMapSize() > 0, + nonNullVarlenValCols); } /** + * Creates RowAssembler for chunks with estimated sizes. + * <p> + * RowAssembler will apply optimizations based on chunks sizes estimations. + * * @param schema Row schema. - * @param size Target row size. If the row size is known in advance, it should be provided upfront to avoid - * unnecessary arrays copy. + * @param keyDataSize Key payload size. Estimated upper-bound or zero if unknown. * @param nonNullVarlenKeyCols Number of non-null varlen columns in key chunk. + * @param valDataSize Value data size. Estimated upper-bound or zero if unknown. * @param nonNullVarlenValCols Number of non-null varlen columns in value chunk. */ public RowAssembler( SchemaDescriptor schema, - int size, + int keyDataSize, int nonNullVarlenKeyCols, + int valDataSize, int nonNullVarlenValCols + ) { + this( Review comment: Inconsistent formatting with line 122 ########## File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/ChunkReader.java ########## @@ -0,0 +1,212 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.schema.row; + +import org.apache.ignite.internal.schema.BinaryRow; +import org.apache.ignite.internal.schema.Columns; + +/** + * Chunk reader. + */ +class ChunkReader { + /** Row. */ + protected final BinaryRow row; + + /** Base offset. */ + protected final int baseOff; + + /** Chunk format. */ + private final ChunkFormat format; + + /** Vartable offset. */ + protected int varTblOff; + + /** Data offset. */ + protected int dataOff; + + /** + * @param row Row. + * @param baseOff Chunk base offset. + * @param nullMapLen Null-map size in bytes. + * @param hasVarTable {@code true} if chunk has vartable, {@code false} otherwise. + * @param format Chunk format. + */ + ChunkReader(BinaryRow row, int baseOff, int nullMapLen, boolean hasVarTable, ChunkFormat format) { + this.row = row; + this.baseOff = baseOff; + this.format = format; + varTblOff = nullmapOff() + nullMapLen; + dataOff = varTblOff + (hasVarTable ? format.vartableLength(format.readVartableSize(row, varTblOff)) : 0); + } + + /** + * Reads chunk total length. + * + * @return Chunk length in bytes + */ + int chunkLength() { + return row.readInteger(baseOff); + } + + /** + * Checks the row's null-map for the given column index in the chunk. + * + * @param idx Offset of the column in the chunk. + * @return {@code true} if the column value is {@code null}, {@code false} otherwise. + */ + protected boolean isNull(int idx) { + if (!hasNullmap()) + return false; + + int nullByte = idx / 8; + int posInByte = idx % 8; Review comment: This should be rewritten using bit opts ########## File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/ChunkFormat.java ########## @@ -0,0 +1,299 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.schema.row; + +import org.apache.ignite.internal.schema.BinaryRow; + +/** + * Chunk format. + */ +abstract class ChunkFormat { + /** First two flag bits reserved for format code. */ + public static final int FORMAT_CODE_MASK = 0x03; + + /** Flag indicates key chunk omits vartable. */ + public static final int OMIT_NULL_MAP_FLAG = 1 << 2; + + /** Flag indicates value chunk omits null map. */ + public static final int OMIT_VARTBL_FLAG = 1 << 3; + + /** Writer factory for tiny-sized chunks. */ + private static final ChunkFormat TINY = new ChunkFormat(Byte.BYTES, Byte.BYTES, (byte)1) { + /** {@inheritDoc} */ + @Override void writeVarlenOffset(ExpandableByteBuf buf, int vartblOff, int entryIdx, int off) { + assert off < (1 << 8) && off >= 0 : "Varlen offset overflow: offset=" + off; + + buf.put(vartblOff + vartableEntryOffset(entryIdx), (byte)off); + } + + /** {@inheritDoc} */ + @Override int readVarlenOffset(BinaryRow row, int vartblOff, int entryIdx) { + return row.readByte(vartblOff + vartableEntryOffset(entryIdx)) & 0xFF; + } + + /** {@inheritDoc} */ + @Override void writeVartableSize(ExpandableByteBuf buf, int vartblOff, int size) { + assert size < (1 << 8) && size >= 0 : "Vartable size overflow: size=" + size; + + buf.put(vartblOff, (byte)size); + } + + /** {@inheritDoc} */ + @Override int readVartableSize(BinaryRow row, int vartblOff) { + return row.readByte(vartblOff) & 0xFF; + } + }; + + /** Writer factory for med-sized chunks. */ + private static final ChunkFormat MEDIUM = new ChunkFormat(Short.BYTES, Short.BYTES, (byte)2) { + /** {@inheritDoc} */ + @Override void writeVarlenOffset(ExpandableByteBuf buf, int vartblOff, int entryIdx, int off) { + assert off < (1 << 16) && off >= 0 : "Varlen offset overflow: offset=" + off; + + buf.putShort(vartblOff + vartableEntryOffset(entryIdx), (short)off); + } + + /** {@inheritDoc} */ + @Override int readVarlenOffset(BinaryRow row, int vartblOff, int entryIdx) { + return row.readShort(vartblOff + vartableEntryOffset(entryIdx)) & 0xFFFF; + } + + /** {@inheritDoc} */ + @Override void writeVartableSize(ExpandableByteBuf buf, int vartblOff, int size) { + assert size < (1 << 16) && size >= 0 : "Vartable size overflow: size=" + size; + + buf.putShort(vartblOff, (short)size); + } + + /** {@inheritDoc} */ + @Override int readVartableSize(BinaryRow row, int vartblOff) { + return row.readShort(vartblOff) & 0xFFFF; + } + }; + + /** Writer factory for large-sized chunks. */ + private static final ChunkFormat LARGE = new ChunkFormat(Short.BYTES, Integer.BYTES, (byte)0) { + /** {@inheritDoc} */ + @Override void writeVarlenOffset(ExpandableByteBuf buf, int vartblOff, int entryIdx, int off) { + buf.putInt(vartblOff + vartableEntryOffset(entryIdx), off); + } + + /** {@inheritDoc} */ + @Override int readVarlenOffset(BinaryRow row, int vartblOff, int entryIdx) { + return row.readInteger(vartblOff + vartableEntryOffset(entryIdx)); + } + + /** {@inheritDoc} */ + @Override void writeVartableSize(ExpandableByteBuf buf, int vartblOff, int size) { + assert size < (1 << 16) && size >= 0 : "Vartable size overflow: size=" + size; + + buf.putShort(vartblOff, (short)size); + } + + /** {@inheritDoc} */ + @Override int readVartableSize(BinaryRow row, int vartblOff) { + return row.readShort(vartblOff) & 0xFFFF; + } + }; + + /** + * Return chunk formatter. + * + * @param payloadLen Payload size in bytes. + * @return Chunk formatter. + */ + static ChunkFormat formatter(int payloadLen) { + if (payloadLen > 0) { + if (payloadLen < 256) + return TINY; + + if (payloadLen < 64 * 1024) + return MEDIUM; + } + + return LARGE; + } + + /** + * Creates chunk reader. + * + * @param row Binary row. + * @param offset Chunk offset. + * @param nullMapSize Default chunk null-map size. + * @param chunkFlags Chunk flags. First 4-bits are meaningful. + * @return Chunk reader. + */ + static ChunkReader createReader(BinaryRow row, int offset, int nullMapSize, byte chunkFlags) { + return fromFlags(chunkFlags).reader(row, offset, nullMapSize, chunkFlags); + } + + /** + * Chunk format factory method. + * + * @param chunkFlags Chunk specific flags. Only first 4-bits are meaningful. + * @return Chunk formatter regarding the provided flags. + */ + private static ChunkFormat fromFlags(byte chunkFlags) { + final int mode = chunkFlags & FORMAT_CODE_MASK; Review comment: This variable is not necessary ########## File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/ChunkFormat.java ########## @@ -0,0 +1,299 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.schema.row; + +import org.apache.ignite.internal.schema.BinaryRow; + +/** + * Chunk format. + */ +abstract class ChunkFormat { Review comment: I think it will be good for later understanding of how fields are packed to give several examples of how different rows are represented in this scheme. ########## File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/ChunkReader.java ########## @@ -0,0 +1,212 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.schema.row; + +import org.apache.ignite.internal.schema.BinaryRow; +import org.apache.ignite.internal.schema.Columns; + +/** + * Chunk reader. + */ +class ChunkReader { + /** Row. */ + protected final BinaryRow row; + + /** Base offset. */ + protected final int baseOff; + + /** Chunk format. */ + private final ChunkFormat format; + + /** Vartable offset. */ + protected int varTblOff; + + /** Data offset. */ + protected int dataOff; + + /** + * @param row Row. + * @param baseOff Chunk base offset. + * @param nullMapLen Null-map size in bytes. + * @param hasVarTable {@code true} if chunk has vartable, {@code false} otherwise. + * @param format Chunk format. + */ + ChunkReader(BinaryRow row, int baseOff, int nullMapLen, boolean hasVarTable, ChunkFormat format) { + this.row = row; + this.baseOff = baseOff; + this.format = format; + varTblOff = nullmapOff() + nullMapLen; + dataOff = varTblOff + (hasVarTable ? format.vartableLength(format.readVartableSize(row, varTblOff)) : 0); + } + + /** + * Reads chunk total length. + * + * @return Chunk length in bytes + */ + int chunkLength() { + return row.readInteger(baseOff); + } + + /** + * Checks the row's null-map for the given column index in the chunk. + * + * @param idx Offset of the column in the chunk. + * @return {@code true} if the column value is {@code null}, {@code false} otherwise. + */ + protected boolean isNull(int idx) { + if (!hasNullmap()) + return false; + + int nullByte = idx / 8; + int posInByte = idx % 8; + + int map = row.readByte(nullmapOff() + nullByte); + + return (map & (1 << posInByte)) != 0; + } + + /** + * @return Null-map offset + */ + private int nullmapOff() { + return baseOff + Integer.BYTES; + } + + /** + * @return {@code True} if chunk has vartable, {@code false} otherwise. + */ + protected boolean hasVartable() { + return dataOff > varTblOff; + } + + /** + * @return {@code True} if chunk has null-map, {@code false} otherwise. + */ + protected boolean hasNullmap() { + return varTblOff > nullmapOff(); + } + + /** + * Calculates the offset of the fixlen column with the given index in the row. It essentially folds the null-map + * with the column lengths to calculate the size of non-null columns preceding the requested column. + * + * @param cols Columns chunk. + * @param idx Column index in the chunk. + * @return Encoded offset (from the row start) of the requested fixlen column. + */ + int fixlenColumnOffset(Columns cols, int idx) { Review comment: Avoid abbrevs in method names -- 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]
