Phillippko commented on code in PR #6377:
URL: https://github.com/apache/ignite-3/pull/6377#discussion_r2260414679
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/index/sorted/comparator/JitComparatorGenerator.java:
##########
@@ -390,18 +440,18 @@ private static MethodDefinition innerCompare(
)));
}
- CatalogColumnCollation collation = columnCollations.get(i);
- NativeType columnType = columnTypes.get(i);
+ CatalogColumnCollation collation =
options.columnCollations().get(i);
+ NativeType columnType = options.columnTypes().get(i);
// Nullability check.
- if (nullableFlags.get(i)) {
+ if (options.nullableFlags().get(i)) {
body.append(outerInNull.set(equal(outerEntryBaseStart,
outerEntryBaseEnd)));
body.append(innerInNull.set(equal(innerEntryBaseStart,
innerEntryBaseEnd)));
// Usage of "bitwiseAnd" and "bitwiseOr" make generated code
easier to read when decompiled.
body.append(new IfStatement()
.condition(bitwiseAnd(outerInNull, innerInNull))
- .ifTrue(lastIteration ? constantInt(0).ret() :
jump(endOfBlockLabel))
+ .ifTrue(lastIteration && !options.supportPrefixes() ?
constantInt(0).ret() : jump(endOfBlockLabel)) // <-------
Review Comment:
What does ```// <-------``` mean?
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/index/sorted/comparator/JitComparatorGenerator.java:
##########
@@ -359,9 +394,24 @@ private static MethodDefinition innerCompare(
Variable outerEntryBaseStart = scope.declareVariable(int.class,
"outerEntryBaseStart");
Variable innerEntryBaseStart = scope.declareVariable(int.class,
"innerEntryBaseStart");
+ Variable prefixColumns = scope.declareVariable(int.class,
"prefixColumns");
+ if (options.supportPrefixes()) {
+ // Here we calculate the number of columns that the prefix has (if
we got a prefix).
+ body.append(new IfStatement()
+ .condition(outerIsPrefix)
+ .ifTrue(new BytecodeBlock()
+ // "outerSize" is reduced by 4. This variable is
later used as an "end" of the last column.
+ .append(outerSize.set(subtract(outerSize,
constantInt(Integer.BYTES))))
Review Comment:
This comment explains code, but doesn't explain it's meaning. Why we reduce
it by Integer.BYTES?
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/index/sorted/comparator/JitComparatorGenerator.java:
##########
@@ -211,18 +217,31 @@ public static JitComparator createComparator(
Variable outerEntrySize = scope.declareVariable(int.class,
"outerEntrySize");
Variable innerEntrySize = scope.declareVariable(int.class,
"innerEntrySize");
+ // Variables for outer flag and its "isPrefix" state.
+ // The latter will only be used if "options.supportPrefixes()" is
"true".
+ Variable outerFlag = scope.declareVariable(byte.class, "outerFlag");
+ Variable outerIsPrefix = scope.declareVariable(boolean.class,
"outerIsPrefix");
+
+ if (options.supportPrefixes() || maxEntrySizeLog != 0) {
+ // We only read outer accessor header if we need its bits.
+ // Otherwise we optimize the code by avoiding this read.
+ body.append(outerFlag.set(outerAccessor.invoke("get", byte.class,
constantInt(0))));
+ }
+
+ if (options.supportPrefixes()) {
+
body.append(outerIsPrefix.set(notEqual(bitwiseAnd(outerFlag.cast(int.class),
constantInt(PREFIX_FLAG)), constantInt(0))));
+ }
+
if (maxEntrySizeLog != 0) {
// If entry size can be larger than 1 byte (and its logarithm
larger than 0), then we read it from headers of tuples like this:
// int outerEntrySize = outerAccessor.get(0) &
BinaryTupleCommon.VARSIZE_MASK;
// Here "Size" still means logarithmic scale.
- Variable outerFlag = scope.declareVariable(byte.class,
"outerFlag");
Variable innerFlag = scope.declareVariable(byte.class,
"innerFlag");
- body.append(outerFlag.set(outerAccessor.invoke("get", byte.class,
constantInt(0))));
body.append(innerFlag.set(innerAccessor.invoke("get", byte.class,
constantInt(0))));
-
body.append(outerEntrySize.set(bitwiseAnd(outerFlag.cast(int.class),
constantInt(BinaryTupleCommon.VARSIZE_MASK))));
-
body.append(innerEntrySize.set(bitwiseAnd(innerFlag.cast(int.class),
constantInt(BinaryTupleCommon.VARSIZE_MASK))));
+
body.append(outerEntrySize.set(bitwiseAnd(outerFlag.cast(int.class),
constantInt(VARSIZE_MASK))));
Review Comment:
Could we call them outerHeader / innerHeader? Or outer/innerFlags?
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/index/sorted/comparator/JitComparatorGenerator.java:
##########
@@ -433,11 +483,32 @@ private static MethodDefinition innerCompare(
body.append(endOfBlockLabel);
- body.append(outerEntryBaseStart.set(outerEntryBaseEnd));
- body.append(innerEntryBaseStart.set(innerEntryBaseEnd));
+ if (!lastIteration) {
+ body.append(outerEntryBaseStart.set(outerEntryBaseEnd));
+ body.append(innerEntryBaseStart.set(innerEntryBaseEnd));
+ }
+ }
+
+ if (options.supportPrefixes()) {
+ // If prefixes are supported, we must always check what column
we compare,
+ // and return a corresponding value if it was the last column
in the prefix.
+ BytecodeExpression outerFlagExpression =
outerAccessor.invoke("get", byte.class, constantInt(0)).cast(int.class);
Review Comment:
Why we read flags again, if we already do it in createComparator method?
--
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: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
