Re: [PR] IGNITE-25909 SQL Calcite: Optimize memory consumption for hash-table … [ignite]
asf-gitbox-commits closed pull request #12189: IGNITE-25909 SQL Calcite: Optimize memory consumption for hash-table … URL: https://github.com/apache/ignite/pull/12189 -- 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]
Re: [PR] IGNITE-25909 SQL Calcite: Optimize memory consumption for hash-table … [ignite]
alex-plekhanov commented on code in PR #12189:
URL: https://github.com/apache/ignite/pull/12189#discussion_r2228374527
##
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/agg/GroupKey.java:
##
@@ -17,86 +17,98 @@
package org.apache.ignite.internal.processors.query.calcite.exec.exp.agg;
-import java.util.Arrays;
-import org.apache.ignite.internal.util.typedef.X;
+import java.util.Objects;
+import org.apache.ignite.binary.BinaryObjectException;
+import org.apache.ignite.binary.BinaryRawReader;
+import org.apache.ignite.binary.BinaryRawWriter;
+import org.apache.ignite.binary.BinaryReader;
+import org.apache.ignite.binary.BinaryWriter;
+import org.apache.ignite.binary.Binarylizable;
+import
org.apache.ignite.internal.processors.query.calcite.exec.ArrayRowHandler;
+import org.apache.ignite.internal.processors.query.calcite.exec.RowHandler;
/**
*
*/
-public class GroupKey {
+public class GroupKey implements Binarylizable {
/** */
-public static final GroupKey EMPTY_GRP_KEY = new
GroupKey(X.EMPTY_OBJECT_ARRAY);
+private Row row;
/** */
-private final Object[] fields;
+private RowHandler hnd;
/** */
-public GroupKey(Object[] fields) {
-this.fields = fields;
+public GroupKey(Row row, RowHandler hnd) {
+this.row = row;
+this.hnd = hnd;
}
/** */
-public Object[] fields() {
-return fields;
+public Row row() {
+return row;
+}
+
+/** */
+public RowHandler rowHandler() {
+return hnd;
}
/** {@inheritDoc} */
-@Override public boolean equals(Object o) {
-if (this == o)
-return true;
-if (o == null || getClass() != o.getClass())
-return false;
+@Override public void writeBinary(BinaryWriter writer) throws
BinaryObjectException {
+BinaryRawWriter rawWriter = writer.rawWriter();
-GroupKey grpKey = (GroupKey)o;
+int colCnt = hnd.columnCount(row);
-return Arrays.equals(fields, grpKey.fields);
+rawWriter.writeInt(colCnt);
+
+for (int i = 0; i < colCnt; i++)
+rawWriter.writeObject(hnd.get(i, row));
}
/** {@inheritDoc} */
-@Override public int hashCode() {
-return Arrays.hashCode(fields);
-}
+@Override public void readBinary(BinaryReader reader) throws
BinaryObjectException {
+BinaryRawReader rawReader = reader.rawReader();
-/** */
-public static Builder builder(int rowLen) {
-return new Builder(rowLen);
-}
+int colCnt = rawReader.readInt();
-/** */
-public static class Builder {
-/** */
-private final Object[] fields;
+Object[] row0 = new Object[colCnt];
-/** */
-private int idx;
+for (int i = 0; i < colCnt; i++)
+row0[i] = rawReader.readObject();
-/** */
-private Builder(int rowLen) {
-fields = new Object[rowLen];
-}
+row = (Row)row0;
+hnd = (RowHandler)ArrayRowHandler.INSTANCE;
+}
-/** */
-public Builder add(Object val) {
-if (idx == fields.length)
-throw new IndexOutOfBoundsException();
+/** {@inheritDoc} */
+@Override public boolean equals(Object o) {
+if (this == o)
+return true;
+if (o == null || getClass() != o.getClass())
+return false;
-fields[idx++] = val;
+GroupKey other = (GroupKey)o;
-return this;
-}
+int colCnt = hnd.columnCount(row);
-/** */
-public GroupKey build() {
-assert idx == fields.length;
+if (colCnt != other.hnd.columnCount(other.row))
+return false;
-return new GroupKey(fields);
+for (int i = 0; i < colCnt; i++) {
+if (!Objects.equals(hnd.get(i, row), other.hnd.get(i, other.row)))
+return false;
}
-/** */
-public void clear() {
-Arrays.fill(fields, null);
+return true;
+}
-idx = 0;
-}
+/** {@inheritDoc} */
+@Override public int hashCode() {
Review Comment:
Row is immutable. Caching hash code will not bring any benefits. This field
is calculated only once - when we put object to hashmap (this statement also
checked by JMH - no performance benefits, only additional memory usage)
--
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]
Re: [PR] IGNITE-25909 SQL Calcite: Optimize memory consumption for hash-table … [ignite]
alex-plekhanov commented on code in PR #12189:
URL: https://github.com/apache/ignite/pull/12189#discussion_r2228363827
##
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/agg/GroupKey.java:
##
@@ -17,86 +17,98 @@
package org.apache.ignite.internal.processors.query.calcite.exec.exp.agg;
-import java.util.Arrays;
-import org.apache.ignite.internal.util.typedef.X;
+import java.util.Objects;
+import org.apache.ignite.binary.BinaryObjectException;
+import org.apache.ignite.binary.BinaryRawReader;
+import org.apache.ignite.binary.BinaryRawWriter;
+import org.apache.ignite.binary.BinaryReader;
+import org.apache.ignite.binary.BinaryWriter;
+import org.apache.ignite.binary.Binarylizable;
+import
org.apache.ignite.internal.processors.query.calcite.exec.ArrayRowHandler;
+import org.apache.ignite.internal.processors.query.calcite.exec.RowHandler;
/**
*
*/
-public class GroupKey {
+public class GroupKey implements Binarylizable {
/** */
-public static final GroupKey EMPTY_GRP_KEY = new
GroupKey(X.EMPTY_OBJECT_ARRAY);
+private Row row;
/** */
-private final Object[] fields;
+private RowHandler hnd;
/** */
-public GroupKey(Object[] fields) {
-this.fields = fields;
+public GroupKey(Row row, RowHandler hnd) {
+this.row = row;
+this.hnd = hnd;
}
/** */
-public Object[] fields() {
-return fields;
+public Row row() {
+return row;
+}
+
+/** */
+public RowHandler rowHandler() {
+return hnd;
}
/** {@inheritDoc} */
-@Override public boolean equals(Object o) {
-if (this == o)
-return true;
-if (o == null || getClass() != o.getClass())
-return false;
+@Override public void writeBinary(BinaryWriter writer) throws
BinaryObjectException {
+BinaryRawWriter rawWriter = writer.rawWriter();
-GroupKey grpKey = (GroupKey)o;
+int colCnt = hnd.columnCount(row);
-return Arrays.equals(fields, grpKey.fields);
+rawWriter.writeInt(colCnt);
+
+for (int i = 0; i < colCnt; i++)
+rawWriter.writeObject(hnd.get(i, row));
}
/** {@inheritDoc} */
-@Override public int hashCode() {
-return Arrays.hashCode(fields);
-}
+@Override public void readBinary(BinaryReader reader) throws
BinaryObjectException {
+BinaryRawReader rawReader = reader.rawReader();
-/** */
-public static Builder builder(int rowLen) {
-return new Builder(rowLen);
-}
+int colCnt = rawReader.readInt();
-/** */
-public static class Builder {
-/** */
-private final Object[] fields;
+Object[] row0 = new Object[colCnt];
-/** */
-private int idx;
+for (int i = 0; i < colCnt; i++)
+row0[i] = rawReader.readObject();
-/** */
-private Builder(int rowLen) {
-fields = new Object[rowLen];
-}
+row = (Row)row0;
+hnd = (RowHandler)ArrayRowHandler.INSTANCE;
Review Comment:
We use different row handlers in constructor. To optimaze serialization we
store only required columns in GroupKey own format. After deserialization we
have new row (as array) and not binded to the old row handler anymore, and can
use array row handler.
--
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]
Re: [PR] IGNITE-25909 SQL Calcite: Optimize memory consumption for hash-table … [ignite]
Vladsz83 commented on code in PR #12189:
URL: https://github.com/apache/ignite/pull/12189#discussion_r2228189537
##
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/agg/GroupKey.java:
##
@@ -17,86 +17,98 @@
package org.apache.ignite.internal.processors.query.calcite.exec.exp.agg;
-import java.util.Arrays;
-import org.apache.ignite.internal.util.typedef.X;
+import java.util.Objects;
+import org.apache.ignite.binary.BinaryObjectException;
+import org.apache.ignite.binary.BinaryRawReader;
+import org.apache.ignite.binary.BinaryRawWriter;
+import org.apache.ignite.binary.BinaryReader;
+import org.apache.ignite.binary.BinaryWriter;
+import org.apache.ignite.binary.Binarylizable;
+import
org.apache.ignite.internal.processors.query.calcite.exec.ArrayRowHandler;
+import org.apache.ignite.internal.processors.query.calcite.exec.RowHandler;
/**
*
*/
-public class GroupKey {
+public class GroupKey implements Binarylizable {
/** */
-public static final GroupKey EMPTY_GRP_KEY = new
GroupKey(X.EMPTY_OBJECT_ARRAY);
+private Row row;
/** */
-private final Object[] fields;
+private RowHandler hnd;
/** */
-public GroupKey(Object[] fields) {
-this.fields = fields;
+public GroupKey(Row row, RowHandler hnd) {
+this.row = row;
+this.hnd = hnd;
}
/** */
-public Object[] fields() {
-return fields;
+public Row row() {
+return row;
+}
+
+/** */
+public RowHandler rowHandler() {
+return hnd;
}
/** {@inheritDoc} */
-@Override public boolean equals(Object o) {
-if (this == o)
-return true;
-if (o == null || getClass() != o.getClass())
-return false;
+@Override public void writeBinary(BinaryWriter writer) throws
BinaryObjectException {
+BinaryRawWriter rawWriter = writer.rawWriter();
-GroupKey grpKey = (GroupKey)o;
+int colCnt = hnd.columnCount(row);
-return Arrays.equals(fields, grpKey.fields);
+rawWriter.writeInt(colCnt);
+
+for (int i = 0; i < colCnt; i++)
+rawWriter.writeObject(hnd.get(i, row));
}
/** {@inheritDoc} */
-@Override public int hashCode() {
-return Arrays.hashCode(fields);
-}
+@Override public void readBinary(BinaryReader reader) throws
BinaryObjectException {
+BinaryRawReader rawReader = reader.rawReader();
-/** */
-public static Builder builder(int rowLen) {
-return new Builder(rowLen);
-}
+int colCnt = rawReader.readInt();
-/** */
-public static class Builder {
-/** */
-private final Object[] fields;
+Object[] row0 = new Object[colCnt];
-/** */
-private int idx;
+for (int i = 0; i < colCnt; i++)
+row0[i] = rawReader.readObject();
-/** */
-private Builder(int rowLen) {
-fields = new Object[rowLen];
-}
+row = (Row)row0;
+hnd = (RowHandler)ArrayRowHandler.INSTANCE;
+}
-/** */
-public Builder add(Object val) {
-if (idx == fields.length)
-throw new IndexOutOfBoundsException();
+/** {@inheritDoc} */
+@Override public boolean equals(Object o) {
+if (this == o)
+return true;
+if (o == null || getClass() != o.getClass())
+return false;
-fields[idx++] = val;
+GroupKey other = (GroupKey)o;
-return this;
-}
+int colCnt = hnd.columnCount(row);
-/** */
-public GroupKey build() {
-assert idx == fields.length;
+if (colCnt != other.hnd.columnCount(other.row))
+return false;
-return new GroupKey(fields);
+for (int i = 0; i < colCnt; i++) {
+if (!Objects.equals(hnd.get(i, row), other.hnd.get(i, other.row)))
+return false;
}
-/** */
-public void clear() {
-Arrays.fill(fields, null);
+return true;
+}
-idx = 0;
-}
+/** {@inheritDoc} */
+@Override public int hashCode() {
Review Comment:
Can be cached? Or row may change?
--
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]
Re: [PR] IGNITE-25909 SQL Calcite: Optimize memory consumption for hash-table … [ignite]
Vladsz83 commented on code in PR #12189:
URL: https://github.com/apache/ignite/pull/12189#discussion_r2228007989
##
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/agg/GroupKey.java:
##
@@ -17,86 +17,98 @@
package org.apache.ignite.internal.processors.query.calcite.exec.exp.agg;
-import java.util.Arrays;
-import org.apache.ignite.internal.util.typedef.X;
+import java.util.Objects;
+import org.apache.ignite.binary.BinaryObjectException;
+import org.apache.ignite.binary.BinaryRawReader;
+import org.apache.ignite.binary.BinaryRawWriter;
+import org.apache.ignite.binary.BinaryReader;
+import org.apache.ignite.binary.BinaryWriter;
+import org.apache.ignite.binary.Binarylizable;
+import
org.apache.ignite.internal.processors.query.calcite.exec.ArrayRowHandler;
+import org.apache.ignite.internal.processors.query.calcite.exec.RowHandler;
/**
*
*/
-public class GroupKey {
+public class GroupKey implements Binarylizable {
/** */
-public static final GroupKey EMPTY_GRP_KEY = new
GroupKey(X.EMPTY_OBJECT_ARRAY);
+private Row row;
/** */
-private final Object[] fields;
+private RowHandler hnd;
/** */
-public GroupKey(Object[] fields) {
-this.fields = fields;
+public GroupKey(Row row, RowHandler hnd) {
+this.row = row;
+this.hnd = hnd;
}
/** */
-public Object[] fields() {
-return fields;
+public Row row() {
+return row;
+}
+
+/** */
+public RowHandler rowHandler() {
+return hnd;
}
/** {@inheritDoc} */
-@Override public boolean equals(Object o) {
-if (this == o)
-return true;
-if (o == null || getClass() != o.getClass())
-return false;
+@Override public void writeBinary(BinaryWriter writer) throws
BinaryObjectException {
+BinaryRawWriter rawWriter = writer.rawWriter();
-GroupKey grpKey = (GroupKey)o;
+int colCnt = hnd.columnCount(row);
-return Arrays.equals(fields, grpKey.fields);
+rawWriter.writeInt(colCnt);
+
+for (int i = 0; i < colCnt; i++)
+rawWriter.writeObject(hnd.get(i, row));
}
/** {@inheritDoc} */
-@Override public int hashCode() {
-return Arrays.hashCode(fields);
-}
+@Override public void readBinary(BinaryReader reader) throws
BinaryObjectException {
+BinaryRawReader rawReader = reader.rawReader();
-/** */
-public static Builder builder(int rowLen) {
-return new Builder(rowLen);
-}
+int colCnt = rawReader.readInt();
-/** */
-public static class Builder {
-/** */
-private final Object[] fields;
+Object[] row0 = new Object[colCnt];
-/** */
-private int idx;
+for (int i = 0; i < colCnt; i++)
+row0[i] = rawReader.readObject();
-/** */
-private Builder(int rowLen) {
-fields = new Object[rowLen];
-}
+row = (Row)row0;
+hnd = (RowHandler)ArrayRowHandler.INSTANCE;
Review Comment:
Can we always use `ArrayRowHandler.INSTANCE` for now ? We pass it in the
constructor, but forcibly set `ArrayRowHandler.INSTANCE` here.
--
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]
Re: [PR] IGNITE-25909 SQL Calcite: Optimize memory consumption for hash-table … [ignite]
sonarqubecloud[bot] commented on PR #12189: URL: https://github.com/apache/ignite/pull/12189#issuecomment-3077477841 ## [](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12189) **Quality Gate failed** Failed conditions  [2 New Code Smells](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12189) (required ≤ 1) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12189) ##  Catch issues before they fail your Quality Gate with our IDE extension  [SonarQube for IDE](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=pull-request) -- 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]
