You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by kiszk <gi...@git.apache.org> on 2018/07/29 17:52:38 UTC
[GitHub] spark pull request #21912: [WIP][SPARK-24962][SQL] Refactor CodeGenerator.cr...
GitHub user kiszk opened a pull request:
https://github.com/apache/spark/pull/21912
[WIP][SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeArray
## What changes were proposed in this pull request?
This PR integrates handling of `UnsafeArrayData` and `GenericArrayData into one, which is a pair of allocation and assignment. The current `CodeGenerator.createUnsafeArray` handles only allocation of `UnsafeArrayData`.
This PR introduces a new method `createArrayData` that returns a code to allocate `UnsafeArrayData` or `GenericArrayData` and to assign a value into the allocated array.
This PR also reduce the size of generated code by calling a runtime helper.
As a first step, several places where `createUnsafeArray` is used straightforward are updated by using `createArrayData`.
ToDos
1. Apply `createArrayData` to other places where `createUnsafeArray` is used
1. Merge `createUnsafeArrayWithFallback` into `createArrayData`
1. Use `createArrayData` in `GenArrayData.genCodeToCreateArrayData()`
## How was this patch tested?
Existing tests
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kiszk/spark SPARK-24962
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/21912.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #21912
----
commit 043aa9d4a587ad904ab2e3af740dd209fbb3ac8a
Author: Kazuaki Ishizaki <is...@...>
Date: 2018-07-29T17:39:40Z
initial commit
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r209855565
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/ArrayData.scala ---
@@ -34,6 +36,37 @@ object ArrayData {
case a: Array[Double] => UnsafeArrayData.fromPrimitiveArray(a)
case other => new GenericArrayData(other)
}
+
+
+ /**
+ * Allocate [[UnsafeArrayData]] or [[GenericArrayData]] based on given parameters.
+ *
+ * @param elementSize a size of an element in bytes
+ * @param numElements the number of elements the array should contain
+ * @param isPrimitiveType whether the type of an element is primitive type
+ * @param additionalErrorMessage string to include in the error message
+ */
+ def allocateArrayData(
+ elementSize: Int,
+ numElements : Long,
+ isPrimitiveType: Boolean,
+ additionalErrorMessage: String) : ArrayData = {
+ val arraySize = UnsafeArrayData.calculateSizeOfUnderlyingByteArray(numElements, elementSize)
+ if (isPrimitiveType && !UnsafeArrayData.shouldUseGenericArrayData(elementSize, numElements)) {
+ val arrayBytes = new Array[Byte](arraySize.toInt)
+ val arrayName = new UnsafeArrayData()
+ Platform.putLong(arrayBytes, Platform.BYTE_ARRAY_OFFSET, numElements)
+ arrayName.pointTo(arrayBytes, Platform.BYTE_ARRAY_OFFSET, arraySize.toInt)
+ arrayName
--- End diff --
Can't we use `UnsafeArrayData.forPrimitiveArray()`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/94933/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #95197 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95197/testReport)** for PR 21912 at commit [`92428cd`](https://github.com/apache/spark/commit/92428cdad9757df9e48b726109da8c3edfa299e5).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r210539249
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -735,70 +735,92 @@ class CodegenContext {
}
/**
- * Generates code creating a [[UnsafeArrayData]].
+ * Generates code of setter for an [[ArrayData]]
+ *
+ * @param elementType data type of the elements in an [[ArrayData]]
+ *
+ * @return code representing a setter of an assignment for an [[ArrayData]]
+ */
+ def setArrayElementFunc(dataType: DataType): String = {
--- End diff --
We should have this in `object CodeGenerator`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #95433 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95433/testReport)** for PR 21912 at commit [`ddc35ac`](https://github.com/apache/spark/commit/ddc35acd2c5394a2b6aea16325172eda9578b4e9).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95199/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2681/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r213884198
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala ---
@@ -385,107 +385,124 @@ case class MapEntries(child: Expression) extends UnaryExpression with ExpectsInp
override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
nullSafeCodeGen(ctx, ev, c => {
+ val arrayData = ctx.freshName("arrayData")
val numElements = ctx.freshName("numElements")
val keys = ctx.freshName("keys")
val values = ctx.freshName("values")
val isKeyPrimitive = CodeGenerator.isPrimitiveType(childDataType.keyType)
val isValuePrimitive = CodeGenerator.isPrimitiveType(childDataType.valueType)
+
+ val wordSize = UnsafeRow.WORD_SIZE
+ val structSize = UnsafeRow.calculateBitSetWidthInBytes(2) + wordSize * 2
+ val (isPrimitive, elementSize) = if (isKeyPrimitive && isValuePrimitive) {
+ (false, structSize + wordSize)
--- End diff --
should be `true`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r213540247
--- Diff: sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/UnsafeArrayData.java ---
@@ -473,13 +473,13 @@ public static UnsafeArrayData fromPrimitiveArray(
return result;
}
- public static UnsafeArrayData forPrimitiveArray(int offset, int length, int elementSize) {
- return fromPrimitiveArray(null, offset, length, elementSize);
+ public static UnsafeArrayData forPrimitiveArray(int length, int elementSize) {
+ return fromPrimitiveArray(null, 0, length, elementSize);
--- End diff --
so this is used to create a new unsafe array with no data. This saves duplicated code, but we will do unnecessary memory copy.
Shall we just add a new method in `UnsafeArrayData` to create a fresh array?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #94934 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94934/testReport)** for PR 21912 at commit [`c0cec34`](https://github.com/apache/spark/commit/c0cec340f74c86a96f67d975ce1bd4ff43021a1e).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/94653/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2521/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [WIP][SPARK-24962][SQL] Refactor CodeGenerator.createUns...
Posted by holdensmagicalunicorn <gi...@git.apache.org>.
Github user holdensmagicalunicorn commented on the issue:
https://github.com/apache/spark/pull/21912
@kiszk, thanks! I am a bot who has found some folks who might be able to help with the review:@ueshin, @cloud-fan and @vanzin
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2732/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95552/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r213883747
--- Diff: sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/UnsafeArrayData.java ---
@@ -452,6 +452,16 @@ public UnsafeArrayData copy() {
public static UnsafeArrayData fromPrimitiveArray(
Object arr, int offset, int length, int elementSize) {
+ UnsafeArrayData result = forPrimitiveArray(length, elementSize);
+ final long headerInBytes = calculateHeaderPortionInBytes(length);
+ final long valueRegionInBytes = (long)elementSize * length;
+ final Object data = result.getBaseObject();
+ Platform.copyMemory(arr, offset, data,
+ Platform.LONG_ARRAY_OFFSET + headerInBytes, valueRegionInBytes);
+ return result;
+ }
+
+ public static UnsafeArrayData forPrimitiveArray(int length, int elementSize) {
--- End diff --
how about `createFreshArray`? It's not for primitive only, e.g. we use it to store a struct of fixed size fields.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r213707288
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala ---
@@ -385,107 +385,120 @@ case class MapEntries(child: Expression) extends UnaryExpression with ExpectsInp
override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
nullSafeCodeGen(ctx, ev, c => {
+ val arrayData = ctx.freshName("arrayData")
val numElements = ctx.freshName("numElements")
val keys = ctx.freshName("keys")
val values = ctx.freshName("values")
val isKeyPrimitive = CodeGenerator.isPrimitiveType(childDataType.keyType)
val isValuePrimitive = CodeGenerator.isPrimitiveType(childDataType.valueType)
+
+ val wordSize = UnsafeRow.WORD_SIZE
+ val structSize = UnsafeRow.calculateBitSetWidthInBytes(2) + wordSize * 2
+ val elementSize = if (isKeyPrimitive && isValuePrimitive) {
+ Some(structSize + wordSize)
+ } else {
+ None
+ }
+
+ val allocation = CodeGenerator.createArrayData(arrayData, childDataType.keyType, numElements,
+ s" $prettyName failed.", elementSize = elementSize)
+
val code = if (isKeyPrimitive && isValuePrimitive) {
- genCodeForPrimitiveElements(ctx, keys, values, ev.value, numElements)
+ val genCodeForPrimitive = genCodeForPrimitiveElements(
+ ctx, arrayData, keys, values, ev.value, numElements, structSize)
+ s"""
+ |if ($arrayData instanceof UnsafeArrayData) {
+ | $genCodeForPrimitive
+ |} else {
+ | ${genCodeForAnyElements(ctx, arrayData, keys, values, ev.value, numElements)}
+ |}
+ """.stripMargin
} else {
- genCodeForAnyElements(ctx, keys, values, ev.value, numElements)
+ s"${genCodeForAnyElements(ctx, arrayData, keys, values, ev.value, numElements)}"
}
+
s"""
|final int $numElements = $c.numElements();
|final ArrayData $keys = $c.keyArray();
|final ArrayData $values = $c.valueArray();
+ |$allocation
|$code
""".stripMargin
})
}
- private def getKey(varName: String) = CodeGenerator.getValue(varName, childDataType.keyType, "z")
+ private def getKey(varName: String, index: String) =
+ CodeGenerator.getValue(varName, childDataType.keyType, index)
- private def getValue(varName: String) = {
- CodeGenerator.getValue(varName, childDataType.valueType, "z")
- }
+ private def getValue(varName: String, index: String) =
+ CodeGenerator.getValue(varName, childDataType.valueType, index)
private def genCodeForPrimitiveElements(
ctx: CodegenContext,
+ arrayData: String,
keys: String,
values: String,
- arrayData: String,
- numElements: String): String = {
- val unsafeRow = ctx.freshName("unsafeRow")
+ resultArrayData: String,
+ numElements: String,
+ structSize: Int): String = {
val unsafeArrayData = ctx.freshName("unsafeArrayData")
+ val baseObject = ctx.freshName("baseObject")
+ val unsafeRow = ctx.freshName("unsafeRow")
val structsOffset = ctx.freshName("structsOffset")
+ val offset = ctx.freshName("offset")
+ val z = ctx.freshName("z")
val calculateHeader = "UnsafeArrayData.calculateHeaderPortionInBytes"
val baseOffset = Platform.BYTE_ARRAY_OFFSET
val wordSize = UnsafeRow.WORD_SIZE
- val structSize = UnsafeRow.calculateBitSetWidthInBytes(2) + wordSize * 2
- val structSizeAsLong = structSize + "L"
- val keyTypeName = CodeGenerator.primitiveTypeName(childDataType.keyType)
- val valueTypeName = CodeGenerator.primitiveTypeName(childDataType.valueType)
-
- val valueAssignment = s"$unsafeRow.set$valueTypeName(1, ${getValue(values)});"
- val valueAssignmentChecked = if (childDataType.valueContainsNull) {
- s"""
- |if ($values.isNullAt(z)) {
- | $unsafeRow.setNullAt(1);
- |} else {
- | $valueAssignment
- |}
- """.stripMargin
- } else {
- valueAssignment
- }
+ val structSizeAsLong = s"${structSize}L"
- val assignmentLoop = (byteArray: String) =>
- s"""
- |final int $structsOffset = $calculateHeader($numElements) + $numElements * $wordSize;
- |UnsafeRow $unsafeRow = new UnsafeRow(2);
- |for (int z = 0; z < $numElements; z++) {
- | long offset = $structsOffset + z * $structSizeAsLong;
- | $unsafeArrayData.setLong(z, (offset << 32) + $structSizeAsLong);
- | $unsafeRow.pointTo($byteArray, $baseOffset + offset, $structSize);
- | $unsafeRow.set$keyTypeName(0, ${getKey(keys)});
- | $valueAssignmentChecked
- |}
- |$arrayData = $unsafeArrayData;
- """.stripMargin
+ val setKey =
+ CodeGenerator.setArrayElement(unsafeRow, childDataType.keyType, "0", getKey(keys, z))
--- End diff --
@cloud-fan Good catch. We will use `setColumn` here.
When I checked source files, it is not straightforward since there are two differences.
1. `value()` is called for `StructType`, `ArrayType`, and others in `setColumn`
2. `setDecimal()` is not supported in `Array`
If we add one boolean value to distinguish `column` and `array`, we can unify them into one. Do we do this unification?
If no, do we update `setColumn` to generate `if` statement for nullcheck like `setArrayElement`? Or, will we update `setColumn` in another PR?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95433/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #94684 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94684/testReport)** for PR 21912 at commit [`c156300`](https://github.com/apache/spark/commit/c15630014c7ef850fa21c1247fa5edd2b1bac81b).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #95552 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95552/testReport)** for PR 21912 at commit [`552a1fa`](https://github.com/apache/spark/commit/552a1fa29e42e0b7a92430a24cad7f34638470bd).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #94933 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94933/testReport)** for PR 21912 at commit [`dd165bd`](https://github.com/apache/spark/commit/dd165bd0e38d5166e92c52ec3a00fd0fd35f7af2).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #94933 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94933/testReport)** for PR 21912 at commit [`dd165bd`](https://github.com/apache/spark/commit/dd165bd0e38d5166e92c52ec3a00fd0fd35f7af2).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95487/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r210543965
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/ArrayData.scala ---
@@ -34,6 +36,32 @@ object ArrayData {
case a: Array[Double] => UnsafeArrayData.fromPrimitiveArray(a)
case other => new GenericArrayData(other)
}
+
+
+ /**
+ * Allocate [[UnsafeArrayData]] or [[GenericArrayData]] based on given parameters.
+ *
+ * @param elementSize a size of an element in bytes
+ * @param numElements the number of elements the array should contain
+ * @param isPrimitiveType whether the type of an element is primitive type
+ * @param additionalErrorMessage string to include in the error message
+ */
+ def allocateArrayData(
+ elementSize: Int,
+ numElements : Long,
+ isPrimitiveType: Boolean,
+ additionalErrorMessage: String) : ArrayData = {
--- End diff --
nit: extra space after `)`.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r209854002
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/ArrayData.scala ---
@@ -34,6 +36,37 @@ object ArrayData {
case a: Array[Double] => UnsafeArrayData.fromPrimitiveArray(a)
case other => new GenericArrayData(other)
}
+
+
+ /**
+ * Allocate [[UnsafeArrayData]] or [[GenericArrayData]] based on given parameters.
+ *
+ * @param elementSize a size of an element in bytes
+ * @param numElements the number of elements the array should contain
+ * @param isPrimitiveType whether the type of an element is primitive type
+ * @param additionalErrorMessage string to include in the error message
+ */
+ def allocateArrayData(
+ elementSize: Int,
+ numElements : Long,
+ isPrimitiveType: Boolean,
+ additionalErrorMessage: String) : ArrayData = {
+ val arraySize = UnsafeArrayData.calculateSizeOfUnderlyingByteArray(numElements, elementSize)
--- End diff --
Move into the following if-clause?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2305/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/21912
cc @cloud-fan
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #95011 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95011/testReport)** for PR 21912 at commit [`5952d51`](https://github.com/apache/spark/commit/5952d5145460cf47534434c388de8f38f7177843).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/21912
cc @ueshin
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r213540868
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -1490,6 +1423,63 @@ object CodeGenerator extends Logging {
}
}
+ /**
+ * Generates code creating a [[UnsafeArrayData]] or [[GenericArrayData]] based on
+ * given parameters.
+ *
+ * @param arrayName name of the array to create
+ * @param elementType data type of the elements in source array
+ * @param numElements code representing the number of elements the array should contain
+ * @param additionalErrorMessage string to include in the error message
+ * @param elementSize optional value which shows the size of an element of the allocated
+ * [[UnsafeArrayData]] or [[GenericArrayData]]
+ *
+ * @return code representing the allocation of [[ArrayData]]
+ */
+ def createArrayData(
+ arrayName: String,
+ elementType: DataType,
+ numElements: String,
+ additionalErrorMessage: String,
+ elementSize: Option[Int] = None): String = {
+ val (isPrimitiveType, elemSize) = if (elementSize.isDefined) {
+ (false, elementSize.get)
+ } else {
+ (CodeGenerator.isPrimitiveType(elementType), elementType.defaultSize)
+ }
+
+ s"""
+ |ArrayData $arrayName = ArrayData.allocateArrayData(
+ | $elemSize, $numElements, $isPrimitiveType, "$additionalErrorMessage");
+ """.stripMargin
+ }
+
+ /**
+ * Generates assignment code for an [[ArrayData]]
--- End diff --
shall we mention that the returned code should be put inside a loop?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r209852203
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -735,70 +735,98 @@ class CodegenContext {
}
/**
- * Generates code creating a [[UnsafeArrayData]].
+ * Generates code creating a [[UnsafeArrayData]] or [[GenericArrayData]] based on
+ * given parameters.
*
* @param arrayName name of the array to create
+ * @param elementType data type of the elements in source array
* @param numElements code representing the number of elements the array should contain
- * @param elementType data type of the elements in the array
* @param additionalErrorMessage string to include in the error message
+ * @param elementSize optional value which shows the size of an element of the allocated
+ * [[UnsafeArrayData]] or [[GenericArrayData]]
+ *
+ * @return code representing the allocation of [[ArrayData]]
+ * code representing a setter of an assignment for the generated array
*/
- def createUnsafeArray(
+ def createArrayData(
--- End diff --
Now that we have `ArrayData.allocateArrayData()`, do we need this method? How about calling it directly instead?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [WIP][SPARK-24962][SQL] Refactor CodeGenerator.createUns...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/1488/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #94653 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94653/testReport)** for PR 21912 at commit [`1a45a16`](https://github.com/apache/spark/commit/1a45a16b9a22d5accb83d00ece704b5eaf4e96c5).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #94938 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94938/testReport)** for PR 21912 at commit [`75f8d45`](https://github.com/apache/spark/commit/75f8d45ba675a69694edfc75e296ae3b185971a0).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [WIP][SPARK-24962][SQL] Refactor CodeGenerator.createUns...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #93786 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/93786/testReport)** for PR 21912 at commit [`9f043dd`](https://github.com/apache/spark/commit/9f043dd276e74ccfde75b8c375a001f45f5897ca).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r214392521
--- Diff: sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/UnsafeArrayData.java ---
@@ -463,23 +463,35 @@ public static UnsafeArrayData fromPrimitiveArray(
final long[] data = new long[(int)totalSizeInLongs];
Platform.putLong(data, Platform.LONG_ARRAY_OFFSET, length);
- if (arr != null) {
--- End diff --
Originally, this if is not here. It is fine with me to keep this for the safety.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r213542353
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala ---
@@ -385,107 +385,120 @@ case class MapEntries(child: Expression) extends UnaryExpression with ExpectsInp
override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
nullSafeCodeGen(ctx, ev, c => {
+ val arrayData = ctx.freshName("arrayData")
val numElements = ctx.freshName("numElements")
val keys = ctx.freshName("keys")
val values = ctx.freshName("values")
val isKeyPrimitive = CodeGenerator.isPrimitiveType(childDataType.keyType)
val isValuePrimitive = CodeGenerator.isPrimitiveType(childDataType.valueType)
+
+ val wordSize = UnsafeRow.WORD_SIZE
+ val structSize = UnsafeRow.calculateBitSetWidthInBytes(2) + wordSize * 2
+ val elementSize = if (isKeyPrimitive && isValuePrimitive) {
+ Some(structSize + wordSize)
+ } else {
+ None
+ }
+
+ val allocation = CodeGenerator.createArrayData(arrayData, childDataType.keyType, numElements,
--- End diff --
this is hacky. Actually we want to create an array of struct, but here we lied and say we want to create an array of key type.
I think we should call `ArrayData.allocateArrayData` here directly.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2368/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #95534 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95534/testReport)** for PR 21912 at commit [`e01fca1`](https://github.com/apache/spark/commit/e01fca1bc65f91f43e9436e579d5670cc4f4a45a).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/1942/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r209574445
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -735,70 +735,100 @@ class CodegenContext {
}
/**
- * Generates code creating a [[UnsafeArrayData]].
+ * Generates code creating a [[UnsafeArrayData]] or [[GenericArrayData]] based on
+ * given parameters.
*
* @param arrayName name of the array to create
+ * @param elementType data type of the elements in source array
* @param numElements code representing the number of elements the array should contain
- * @param elementType data type of the elements in the array
* @param additionalErrorMessage string to include in the error message
+ * @param elementSize optional value which shows the size of an element of the allocated
+ * [[UnsafeArrayData]] or [[GenericArrayData]]
+ *
+ * @return code representing the allocation of [[ArrayData]]
+ * code representing a setter of an assignment for the generated array
*/
- def createUnsafeArray(
+ def createArrayData(
arrayName: String,
- numElements: String,
elementType: DataType,
- additionalErrorMessage: String): String = {
- val arraySize = freshName("size")
- val arrayBytes = freshName("arrayBytes")
+ numElements: String,
+ additionalErrorMessage: String,
+ elementSize: Option[Int] = None): (String, String) = {
+ val isPrimitiveType = if (elementSize.isDefined) {
+ false
+ } else {
+ CodeGenerator.isPrimitiveType(elementType)
+ }
- s"""
- |long $arraySize = UnsafeArrayData.calculateSizeOfUnderlyingByteArray(
- | $numElements,
- | ${elementType.defaultSize});
- |if ($arraySize > ${ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH}) {
- | throw new RuntimeException("Unsuccessful try create array with " + $arraySize +
- | " bytes of data due to exceeding the limit " +
- | "${ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH} bytes for UnsafeArrayData." +
- | "$additionalErrorMessage");
- |}
- |byte[] $arrayBytes = new byte[(int)$arraySize];
- |UnsafeArrayData $arrayName = new UnsafeArrayData();
- |Platform.putLong($arrayBytes, ${Platform.BYTE_ARRAY_OFFSET}, $numElements);
- |$arrayName.pointTo($arrayBytes, ${Platform.BYTE_ARRAY_OFFSET}, (int)$arraySize);
- """.stripMargin
+ val setFunc = if (isPrimitiveType) {
+ s"set${CodeGenerator.primitiveTypeName(elementType)}"
+ } else {
+ "update"
+ }
+
+ val elemSize = if (elementSize.isDefined) {
+ elementSize.get
+ } else {
+ elementType.defaultSize
+ }
+ val arrayData = classOf[ArrayData].getName
+ val allocation =
+ s"""
+ |ArrayData $arrayName = $arrayData$$.MODULE$$.allocateArrayData(
+ | $elemSize, $numElements, $isPrimitiveType, "$additionalErrorMessage");
+ """.stripMargin
+
+ (allocation, setFunc)
}
/**
- * Generates code creating a [[UnsafeArrayData]]. The generated code executes
- * a provided fallback when the size of backing array would exceed the array size limit.
- * @param arrayName a name of the array to create
- * @param numElements a piece of code representing the number of elements the array should contain
- * @param elementSize a size of an element in bytes
- * @param bodyCode a function generating code that fills up the [[UnsafeArrayData]]
- * and getting the backing array as a parameter
- * @param fallbackCode a piece of code executed when the array size limit is exceeded
+ * Generates assignment code for a [[ArrayData]]
+ *
+ * @param arrayName name of the array to create
+ * @param dataType data type of the result array
+ * @param elementType data type of the elements in source array
+ * @param srcArray code representing the number of elements the array should contain
+ * @param setFunc string to include in the error message
+ * @param rhsValue an optionally specified expression for the right-hand side of the returning
+ * assignment
+ * @param checkForNull optional value which shows whether a nullcheck is required for
+ * the returning assignment
+ *
+ * @return code representing an assignment to each element of the [[ArrayData]], which requires
+ * a pair of destination and source loop index variables
*/
- def createUnsafeArrayWithFallback(
+ def createArrayAssignment(
arrayName: String,
- numElements: String,
- elementSize: Int,
- bodyCode: String => String,
- fallbackCode: String): String = {
- val arraySize = freshName("size")
- val arrayBytes = freshName("arrayBytes")
- s"""
- |final long $arraySize = UnsafeArrayData.calculateSizeOfUnderlyingByteArray(
- | $numElements,
- | $elementSize);
- |if ($arraySize > ${ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH}) {
- | $fallbackCode
- |} else {
- | final byte[] $arrayBytes = new byte[(int)$arraySize];
- | UnsafeArrayData $arrayName = new UnsafeArrayData();
- | Platform.putLong($arrayBytes, ${Platform.BYTE_ARRAY_OFFSET}, $numElements);
- | $arrayName.pointTo($arrayBytes, ${Platform.BYTE_ARRAY_OFFSET}, (int)$arraySize);
- | ${bodyCode(arrayBytes)}
- |}
- """.stripMargin
+ dataType: DataType,
+ elementType: DataType,
+ srcArray: String,
+ setFunc: String,
+ rhsValue: String = null,
--- End diff --
if it's optional, we should use `Option[String]`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r209862522
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala ---
@@ -3330,50 +3136,123 @@ case class ArrayRemove(left: Expression, right: Expression)
val pos = ctx.freshName("pos")
val getValue = CodeGenerator.getValue(inputArray, elementType, i)
val isEqual = ctx.genEqual(elementType, value, getValue)
- if (!CodeGenerator.isPrimitiveType(elementType)) {
- val arrayClass = classOf[GenericArrayData].getName
+
+ val (allocation, setFunc) =
+ ctx.createArrayData(values, elementType, newArraySize, s" $prettyName failed.")
+ val assignment = ctx.createArrayAssignment(
+ values, dataType, elementType, inputArray, setFunc, checkForNull = Some(false))
+
+ s"""
+ |$allocation
+ |int $pos = 0;
+ |for (int $i = 0; $i < $inputArray.numElements(); $i ++) {
+ | if ($inputArray.isNullAt($i)) {
+ | $values.setNullAt($pos);
+ | $pos = $pos + 1;
+ | }
+ | else {
+ | if (!($isEqual)) {
+ | ${assignment(pos, i)}
+ | $pos = $pos + 1;
+ | }
+ | }
+ |}
+ |${ev.value} = $values;
+ """.stripMargin
+ }
+
+ override def prettyName: String = "array_remove"
+}
+
+/**
+ * Will become common base class for [[ArrayDistinct]], [[ArrayUnion]], [[ArrayIntersect]],
+ * and [[ArrayExcept]].
+ */
+trait ArraySetLike {
+ @transient protected lazy val dt: DataType = NullType
+ @transient protected lazy val et: DataType = NullType
--- End diff --
How about: `protected def dt: ArrayType` and `protected def et: DataType`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #94684 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94684/testReport)** for PR 21912 at commit [`c156300`](https://github.com/apache/spark/commit/c15630014c7ef850fa21c1247fa5edd2b1bac81b).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #95046 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95046/testReport)** for PR 21912 at commit [`a79d506`](https://github.com/apache/spark/commit/a79d506829c573f1a5c0dc3253d20442d4aa6d90).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #94812 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94812/testReport)** for PR 21912 at commit [`240936e`](https://github.com/apache/spark/commit/240936e98c6b32855236c7c635bacc0fb04acb76).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r209853481
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -735,70 +735,98 @@ class CodegenContext {
}
/**
- * Generates code creating a [[UnsafeArrayData]].
+ * Generates code creating a [[UnsafeArrayData]] or [[GenericArrayData]] based on
+ * given parameters.
*
* @param arrayName name of the array to create
+ * @param elementType data type of the elements in source array
* @param numElements code representing the number of elements the array should contain
- * @param elementType data type of the elements in the array
* @param additionalErrorMessage string to include in the error message
+ * @param elementSize optional value which shows the size of an element of the allocated
+ * [[UnsafeArrayData]] or [[GenericArrayData]]
+ *
+ * @return code representing the allocation of [[ArrayData]]
+ * code representing a setter of an assignment for the generated array
*/
- def createUnsafeArray(
+ def createArrayData(
arrayName: String,
- numElements: String,
elementType: DataType,
- additionalErrorMessage: String): String = {
- val arraySize = freshName("size")
- val arrayBytes = freshName("arrayBytes")
+ numElements: String,
+ additionalErrorMessage: String,
+ elementSize: Option[Int] = None): (String, String) = {
+ val isPrimitiveType = if (elementSize.isDefined) {
+ false
+ } else {
+ CodeGenerator.isPrimitiveType(elementType)
+ }
- s"""
- |long $arraySize = UnsafeArrayData.calculateSizeOfUnderlyingByteArray(
- | $numElements,
- | ${elementType.defaultSize});
- |if ($arraySize > ${ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH}) {
- | throw new RuntimeException("Unsuccessful try create array with " + $arraySize +
- | " bytes of data due to exceeding the limit " +
- | "${ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH} bytes for UnsafeArrayData." +
- | "$additionalErrorMessage");
- |}
- |byte[] $arrayBytes = new byte[(int)$arraySize];
- |UnsafeArrayData $arrayName = new UnsafeArrayData();
- |Platform.putLong($arrayBytes, ${Platform.BYTE_ARRAY_OFFSET}, $numElements);
- |$arrayName.pointTo($arrayBytes, ${Platform.BYTE_ARRAY_OFFSET}, (int)$arraySize);
- """.stripMargin
+ val setFunc = if (isPrimitiveType) {
+ s"set${CodeGenerator.primitiveTypeName(elementType)}"
+ } else {
+ "update"
+ }
--- End diff --
Maybe only this is good to have. `def setArrayElementFunc(dt: DataType): String`?
Or how about `def setArrayElement(array: String, dataType: DataType, ordinal: Int, value: String): String` like other set utilities?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/21912
thanks, merging to master!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r210530380
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -735,70 +735,98 @@ class CodegenContext {
}
/**
- * Generates code creating a [[UnsafeArrayData]].
+ * Generates code creating a [[UnsafeArrayData]] or [[GenericArrayData]] based on
+ * given parameters.
*
* @param arrayName name of the array to create
+ * @param elementType data type of the elements in source array
* @param numElements code representing the number of elements the array should contain
- * @param elementType data type of the elements in the array
* @param additionalErrorMessage string to include in the error message
+ * @param elementSize optional value which shows the size of an element of the allocated
+ * [[UnsafeArrayData]] or [[GenericArrayData]]
+ *
+ * @return code representing the allocation of [[ArrayData]]
+ * code representing a setter of an assignment for the generated array
*/
- def createUnsafeArray(
+ def createArrayData(
--- End diff --
I'm still not sure if it would be good to have this. I think the method is small enough to inline. cc @cloud-fan
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95534/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [WIP][SPARK-24962][SQL] Refactor CodeGenerator.createUns...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #93786 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/93786/testReport)** for PR 21912 at commit [`9f043dd`](https://github.com/apache/spark/commit/9f043dd276e74ccfde75b8c375a001f45f5897ca).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [WIP][SPARK-24962][SQL] Refactor CodeGenerator.createUns...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/93786/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/21912
cc @ueshin
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #94934 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94934/testReport)** for PR 21912 at commit [`c0cec34`](https://github.com/apache/spark/commit/c0cec340f74c86a96f67d975ce1bd4ff43021a1e).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r209854249
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/ArrayData.scala ---
@@ -34,6 +36,37 @@ object ArrayData {
case a: Array[Double] => UnsafeArrayData.fromPrimitiveArray(a)
case other => new GenericArrayData(other)
}
+
+
+ /**
+ * Allocate [[UnsafeArrayData]] or [[GenericArrayData]] based on given parameters.
+ *
+ * @param elementSize a size of an element in bytes
+ * @param numElements the number of elements the array should contain
+ * @param isPrimitiveType whether the type of an element is primitive type
+ * @param additionalErrorMessage string to include in the error message
+ */
+ def allocateArrayData(
+ elementSize: Int,
+ numElements : Long,
+ isPrimitiveType: Boolean,
+ additionalErrorMessage: String) : ArrayData = {
+ val arraySize = UnsafeArrayData.calculateSizeOfUnderlyingByteArray(numElements, elementSize)
+ if (isPrimitiveType && !UnsafeArrayData.shouldUseGenericArrayData(elementSize, numElements)) {
+ val arrayBytes = new Array[Byte](arraySize.toInt)
+ val arrayName = new UnsafeArrayData()
+ Platform.putLong(arrayBytes, Platform.BYTE_ARRAY_OFFSET, numElements)
+ arrayName.pointTo(arrayBytes, Platform.BYTE_ARRAY_OFFSET, arraySize.toInt)
+ arrayName
+ } else if (numElements <= ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH.toLong) {
+ new GenericArrayData(new Array[Any](numElements.toInt))
+ } else {
+ throw new RuntimeException(s"Cannot create array with $numElements " +
+ "elements of data due to exceeding the limit " +
+ s"${ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH} elements for ArrayData." +
+ s"$additionalErrorMessage")
--- End diff --
nit: no need to surround with `s""`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/94421/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [WIP][SPARK-24962][SQL] Refactor CodeGenerator.createUns...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #93753 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/93753/testReport)** for PR 21912 at commit [`043aa9d`](https://github.com/apache/spark/commit/043aa9d4a587ad904ab2e3af740dd209fbb3ac8a).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [WIP][SPARK-24962][SQL] Refactor CodeGenerator.createUns...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/1462/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r214145047
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala ---
@@ -385,107 +385,124 @@ case class MapEntries(child: Expression) extends UnaryExpression with ExpectsInp
override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
nullSafeCodeGen(ctx, ev, c => {
+ val arrayData = ctx.freshName("arrayData")
val numElements = ctx.freshName("numElements")
val keys = ctx.freshName("keys")
val values = ctx.freshName("values")
val isKeyPrimitive = CodeGenerator.isPrimitiveType(childDataType.keyType)
val isValuePrimitive = CodeGenerator.isPrimitiveType(childDataType.valueType)
+
+ val wordSize = UnsafeRow.WORD_SIZE
+ val structSize = UnsafeRow.calculateBitSetWidthInBytes(2) + wordSize * 2
+ val (isPrimitive, elementSize) = if (isKeyPrimitive && isValuePrimitive) {
+ (false, structSize + wordSize)
--- End diff --
Good catch, thank you!!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/21912
cc @ueshin
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95011/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #94810 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94810/testReport)** for PR 21912 at commit [`b899259`](https://github.com/apache/spark/commit/b899259125ed2c7b94c0b121d6cbdeb4ee90285a).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/21912
LGTM except a few comments
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #95552 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95552/testReport)** for PR 21912 at commit [`552a1fa`](https://github.com/apache/spark/commit/552a1fa29e42e0b7a92430a24cad7f34638470bd).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r209877426
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/ArrayData.scala ---
@@ -34,6 +36,37 @@ object ArrayData {
case a: Array[Double] => UnsafeArrayData.fromPrimitiveArray(a)
case other => new GenericArrayData(other)
}
+
+
+ /**
+ * Allocate [[UnsafeArrayData]] or [[GenericArrayData]] based on given parameters.
+ *
+ * @param elementSize a size of an element in bytes
+ * @param numElements the number of elements the array should contain
+ * @param isPrimitiveType whether the type of an element is primitive type
+ * @param additionalErrorMessage string to include in the error message
+ */
+ def allocateArrayData(
+ elementSize: Int,
+ numElements : Long,
+ isPrimitiveType: Boolean,
+ additionalErrorMessage: String) : ArrayData = {
+ val arraySize = UnsafeArrayData.calculateSizeOfUnderlyingByteArray(numElements, elementSize)
+ if (isPrimitiveType && !UnsafeArrayData.shouldUseGenericArrayData(elementSize, numElements)) {
--- End diff --
When `UnsafeArrayData` can be used, `GenericArrayData` is also used. However, if the element size is large, `GenericArrayData` should be used. But, `UnsafeArrayData` cannot be used.
Thus, I think that it would be good to use the current name `shouldUseGenericArrayData`.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #95011 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95011/testReport)** for PR 21912 at commit [`5952d51`](https://github.com/apache/spark/commit/5952d5145460cf47534434c388de8f38f7177843).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [WIP][SPARK-24962][SQL] Refactor CodeGenerator.createUns...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r210531207
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -735,70 +735,92 @@ class CodegenContext {
}
/**
- * Generates code creating a [[UnsafeArrayData]].
+ * Generates code of setter for an [[ArrayData]]
+ *
+ * @param elementType data type of the elements in an [[ArrayData]]
+ *
+ * @return code representing a setter of an assignment for an [[ArrayData]]
+ */
+ def setArrayElementFunc(dataType: DataType): String = {
+ val isPrimitiveType = CodeGenerator.isPrimitiveType(dataType)
+ if (isPrimitiveType) {
+ s"set${CodeGenerator.primitiveTypeName(dataType)}"
+ } else {
+ "update"
+ }
+ }
+
+ /**
+ * Generates code creating a [[UnsafeArrayData]] or [[GenericArrayData]] based on
+ * given parameters.
*
* @param arrayName name of the array to create
+ * @param elementType data type of the elements in source array
* @param numElements code representing the number of elements the array should contain
- * @param elementType data type of the elements in the array
* @param additionalErrorMessage string to include in the error message
+ * @param elementSize optional value which shows the size of an element of the allocated
+ * [[UnsafeArrayData]] or [[GenericArrayData]]
+ *
+ * @return code representing the allocation of [[ArrayData]]
+ * code representing a setter of an assignment for the generated array
*/
- def createUnsafeArray(
+ def createArrayData(
arrayName: String,
- numElements: String,
elementType: DataType,
- additionalErrorMessage: String): String = {
- val arraySize = freshName("size")
- val arrayBytes = freshName("arrayBytes")
+ numElements: String,
+ additionalErrorMessage: String,
+ elementSize: Option[Int] = None): String = {
+ val (isPrimitiveType, elemSize) = if (elementSize.isDefined) {
+ (false, elementSize.get)
+ } else {
+ (CodeGenerator.isPrimitiveType(elementType), elementType.defaultSize)
+ }
s"""
- |long $arraySize = UnsafeArrayData.calculateSizeOfUnderlyingByteArray(
- | $numElements,
- | ${elementType.defaultSize});
- |if ($arraySize > ${ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH}) {
- | throw new RuntimeException("Unsuccessful try create array with " + $arraySize +
- | " bytes of data due to exceeding the limit " +
- | "${ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH} bytes for UnsafeArrayData." +
- | "$additionalErrorMessage");
- |}
- |byte[] $arrayBytes = new byte[(int)$arraySize];
- |UnsafeArrayData $arrayName = new UnsafeArrayData();
- |Platform.putLong($arrayBytes, ${Platform.BYTE_ARRAY_OFFSET}, $numElements);
- |$arrayName.pointTo($arrayBytes, ${Platform.BYTE_ARRAY_OFFSET}, (int)$arraySize);
- """.stripMargin
+ |ArrayData $arrayName = ArrayData.allocateArrayData(
+ | $elemSize, $numElements, $isPrimitiveType, "$additionalErrorMessage");
+ """.stripMargin
}
/**
- * Generates code creating a [[UnsafeArrayData]]. The generated code executes
- * a provided fallback when the size of backing array would exceed the array size limit.
- * @param arrayName a name of the array to create
- * @param numElements a piece of code representing the number of elements the array should contain
- * @param elementSize a size of an element in bytes
- * @param bodyCode a function generating code that fills up the [[UnsafeArrayData]]
- * and getting the backing array as a parameter
- * @param fallbackCode a piece of code executed when the array size limit is exceeded
+ * Generates assignment code for a [[ArrayData]]
+ *
+ * @param arrayName name of the array to create
+ * @param setFunc string to include in the error message
+ * @param elementType data type of the elements in source array
+ * @param srcArray code representing the number of elements the array should contain
+ * @param needNullCheck value which shows whether a nullcheck is required for the returning
+ * assignment
+ * @param rhsValue optional expression for the right-hand side of the returning assignment
+ *
+ * @return code representing an assignment to each element of the [[ArrayData]], which requires
+ * a pair of destination and source loop index variables
*/
- def createUnsafeArrayWithFallback(
+ def createArrayAssignment(
arrayName: String,
- numElements: String,
- elementSize: Int,
- bodyCode: String => String,
- fallbackCode: String): String = {
- val arraySize = freshName("size")
- val arrayBytes = freshName("arrayBytes")
- s"""
- |final long $arraySize = UnsafeArrayData.calculateSizeOfUnderlyingByteArray(
- | $numElements,
- | $elementSize);
- |if ($arraySize > ${ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH}) {
- | $fallbackCode
- |} else {
- | final byte[] $arrayBytes = new byte[(int)$arraySize];
- | UnsafeArrayData $arrayName = new UnsafeArrayData();
- | Platform.putLong($arrayBytes, ${Platform.BYTE_ARRAY_OFFSET}, $numElements);
- | $arrayName.pointTo($arrayBytes, ${Platform.BYTE_ARRAY_OFFSET}, (int)$arraySize);
- | ${bodyCode(arrayBytes)}
- |}
- """.stripMargin
+ setFunc: String,
--- End diff --
Do we need this argument? Now we can know the func from the element type.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r213539696
--- Diff: sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/UnsafeArrayData.java ---
@@ -473,13 +473,13 @@ public static UnsafeArrayData fromPrimitiveArray(
return result;
}
- public static UnsafeArrayData forPrimitiveArray(int offset, int length, int elementSize) {
- return fromPrimitiveArray(null, offset, length, elementSize);
+ public static UnsafeArrayData forPrimitiveArray(int length, int elementSize) {
+ return fromPrimitiveArray(null, 0, length, elementSize);
--- End diff --
is it safe? I vaguely remember the address of offheap memory block is usually not 0.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #95433 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95433/testReport)** for PR 21912 at commit [`ddc35ac`](https://github.com/apache/spark/commit/ddc35acd2c5394a2b6aea16325172eda9578b4e9).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2708/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [WIP][SPARK-24962][SQL] Refactor CodeGenerator.createUns...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #94421 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94421/testReport)** for PR 21912 at commit [`409cdd8`](https://github.com/apache/spark/commit/409cdd8667a423032e90b28f7b0e1487c332b60f).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/94419/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [WIP][SPARK-24962][SQL] Refactor CodeGenerator.createUns...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #95197 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95197/testReport)** for PR 21912 at commit [`92428cd`](https://github.com/apache/spark/commit/92428cdad9757df9e48b726109da8c3edfa299e5).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2306/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r210543909
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/ArrayData.scala ---
@@ -34,6 +36,32 @@ object ArrayData {
case a: Array[Double] => UnsafeArrayData.fromPrimitiveArray(a)
case other => new GenericArrayData(other)
}
+
+
+ /**
+ * Allocate [[UnsafeArrayData]] or [[GenericArrayData]] based on given parameters.
+ *
+ * @param elementSize a size of an element in bytes
+ * @param numElements the number of elements the array should contain
+ * @param isPrimitiveType whether the type of an element is primitive type
+ * @param additionalErrorMessage string to include in the error message
+ */
+ def allocateArrayData(
+ elementSize: Int,
+ numElements : Long,
--- End diff --
nit: extra space after `numElements`.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r213540562
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -1490,6 +1423,63 @@ object CodeGenerator extends Logging {
}
}
+ /**
+ * Generates code creating a [[UnsafeArrayData]] or [[GenericArrayData]] based on
+ * given parameters.
+ *
+ * @param arrayName name of the array to create
+ * @param elementType data type of the elements in source array
+ * @param numElements code representing the number of elements the array should contain
+ * @param additionalErrorMessage string to include in the error message
+ * @param elementSize optional value which shows the size of an element of the allocated
+ * [[UnsafeArrayData]] or [[GenericArrayData]]
+ *
+ * @return code representing the allocation of [[ArrayData]]
+ */
+ def createArrayData(
+ arrayName: String,
+ elementType: DataType,
+ numElements: String,
+ additionalErrorMessage: String,
+ elementSize: Option[Int] = None): String = {
+ val (isPrimitiveType, elemSize) = if (elementSize.isDefined) {
+ (false, elementSize.get)
+ } else {
+ (CodeGenerator.isPrimitiveType(elementType), elementType.defaultSize)
+ }
+
+ s"""
+ |ArrayData $arrayName = ArrayData.allocateArrayData(
+ | $elemSize, $numElements, $isPrimitiveType, "$additionalErrorMessage");
+ """.stripMargin
+ }
+
+ /**
+ * Generates assignment code for an [[ArrayData]]
+ *
+ * @param arrayName name of the array to create
+ * @param elementType data type of the elements in destination and source arrays
+ * @param srcArray code representing the number of elements the array should contain
--- End diff --
rename it to `length`.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #95534 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95534/testReport)** for PR 21912 at commit [`e01fca1`](https://github.com/apache/spark/commit/e01fca1bc65f91f43e9436e579d5670cc4f4a45a).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r214332656
--- Diff: sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/UnsafeArrayData.java ---
@@ -463,23 +463,35 @@ public static UnsafeArrayData fromPrimitiveArray(
final long[] data = new long[(int)totalSizeInLongs];
Platform.putLong(data, Platform.LONG_ARRAY_OFFSET, length);
- if (arr != null) {
--- End diff --
shall we keep this if?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/21912
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95490/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95197/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/94934/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2218/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [WIP][SPARK-24962][SQL] Refactor CodeGenerator.createUns...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #94419 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94419/testReport)** for PR 21912 at commit [`5953668`](https://github.com/apache/spark/commit/5953668d58e216f31d1c3771e5ce116ae1332313).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2129/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #95490 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95490/testReport)** for PR 21912 at commit [`61ea681`](https://github.com/apache/spark/commit/61ea68139125312749415dce65c23f797d54473d).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/94810/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #95199 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95199/testReport)** for PR 21912 at commit [`bfd8788`](https://github.com/apache/spark/commit/bfd878837fd1b8bb0ba29ba892f4d6ead9027e4d).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r210539389
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -735,70 +735,92 @@ class CodegenContext {
}
/**
- * Generates code creating a [[UnsafeArrayData]].
+ * Generates code of setter for an [[ArrayData]]
+ *
+ * @param elementType data type of the elements in an [[ArrayData]]
+ *
+ * @return code representing a setter of an assignment for an [[ArrayData]]
+ */
+ def setArrayElementFunc(dataType: DataType): String = {
+ val isPrimitiveType = CodeGenerator.isPrimitiveType(dataType)
+ if (isPrimitiveType) {
+ s"set${CodeGenerator.primitiveTypeName(dataType)}"
+ } else {
+ "update"
+ }
+ }
+
+ /**
+ * Generates code creating a [[UnsafeArrayData]] or [[GenericArrayData]] based on
+ * given parameters.
*
* @param arrayName name of the array to create
+ * @param elementType data type of the elements in source array
* @param numElements code representing the number of elements the array should contain
- * @param elementType data type of the elements in the array
* @param additionalErrorMessage string to include in the error message
+ * @param elementSize optional value which shows the size of an element of the allocated
+ * [[UnsafeArrayData]] or [[GenericArrayData]]
+ *
+ * @return code representing the allocation of [[ArrayData]]
+ * code representing a setter of an assignment for the generated array
*/
- def createUnsafeArray(
+ def createArrayData(
arrayName: String,
- numElements: String,
elementType: DataType,
- additionalErrorMessage: String): String = {
- val arraySize = freshName("size")
- val arrayBytes = freshName("arrayBytes")
+ numElements: String,
+ additionalErrorMessage: String,
+ elementSize: Option[Int] = None): String = {
+ val (isPrimitiveType, elemSize) = if (elementSize.isDefined) {
+ (false, elementSize.get)
+ } else {
+ (CodeGenerator.isPrimitiveType(elementType), elementType.defaultSize)
+ }
s"""
- |long $arraySize = UnsafeArrayData.calculateSizeOfUnderlyingByteArray(
- | $numElements,
- | ${elementType.defaultSize});
- |if ($arraySize > ${ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH}) {
- | throw new RuntimeException("Unsuccessful try create array with " + $arraySize +
- | " bytes of data due to exceeding the limit " +
- | "${ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH} bytes for UnsafeArrayData." +
- | "$additionalErrorMessage");
- |}
- |byte[] $arrayBytes = new byte[(int)$arraySize];
- |UnsafeArrayData $arrayName = new UnsafeArrayData();
- |Platform.putLong($arrayBytes, ${Platform.BYTE_ARRAY_OFFSET}, $numElements);
- |$arrayName.pointTo($arrayBytes, ${Platform.BYTE_ARRAY_OFFSET}, (int)$arraySize);
- """.stripMargin
+ |ArrayData $arrayName = ArrayData.allocateArrayData(
+ | $elemSize, $numElements, $isPrimitiveType, "$additionalErrorMessage");
+ """.stripMargin
}
/**
- * Generates code creating a [[UnsafeArrayData]]. The generated code executes
- * a provided fallback when the size of backing array would exceed the array size limit.
- * @param arrayName a name of the array to create
- * @param numElements a piece of code representing the number of elements the array should contain
- * @param elementSize a size of an element in bytes
- * @param bodyCode a function generating code that fills up the [[UnsafeArrayData]]
- * and getting the backing array as a parameter
- * @param fallbackCode a piece of code executed when the array size limit is exceeded
+ * Generates assignment code for a [[ArrayData]]
+ *
+ * @param arrayName name of the array to create
+ * @param setFunc string to include in the error message
+ * @param elementType data type of the elements in source array
+ * @param srcArray code representing the number of elements the array should contain
+ * @param needNullCheck value which shows whether a nullcheck is required for the returning
+ * assignment
+ * @param rhsValue optional expression for the right-hand side of the returning assignment
+ *
+ * @return code representing an assignment to each element of the [[ArrayData]], which requires
+ * a pair of destination and source loop index variables
*/
- def createUnsafeArrayWithFallback(
+ def createArrayAssignment(
arrayName: String,
- numElements: String,
- elementSize: Int,
- bodyCode: String => String,
- fallbackCode: String): String = {
- val arraySize = freshName("size")
- val arrayBytes = freshName("arrayBytes")
- s"""
- |final long $arraySize = UnsafeArrayData.calculateSizeOfUnderlyingByteArray(
- | $numElements,
- | $elementSize);
- |if ($arraySize > ${ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH}) {
- | $fallbackCode
- |} else {
- | final byte[] $arrayBytes = new byte[(int)$arraySize];
- | UnsafeArrayData $arrayName = new UnsafeArrayData();
- | Platform.putLong($arrayBytes, ${Platform.BYTE_ARRAY_OFFSET}, $numElements);
- | $arrayName.pointTo($arrayBytes, ${Platform.BYTE_ARRAY_OFFSET}, (int)$arraySize);
- | ${bodyCode(arrayBytes)}
- |}
- """.stripMargin
+ setFunc: String,
+ elementType: DataType,
+ srcArray: String,
+ needNullCheck: Boolean,
+ rhsValue: Option[String] = None): (String, String) => String = {
--- End diff --
ditto.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/21912
cc @cloud-fan
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/21912
cc @ueshin
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/21912
cc @cloud-fan
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #94938 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94938/testReport)** for PR 21912 at commit [`75f8d45`](https://github.com/apache/spark/commit/75f8d45ba675a69694edfc75e296ae3b185971a0).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #95046 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95046/testReport)** for PR 21912 at commit [`a79d506`](https://github.com/apache/spark/commit/a79d506829c573f1a5c0dc3253d20442d4aa6d90).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/94646/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [WIP][SPARK-24962][SQL] Refactor CodeGenerator.createUns...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2106/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #94646 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94646/testReport)** for PR 21912 at commit [`32dfcb5`](https://github.com/apache/spark/commit/32dfcb57b280884e57e464c3d718498510c9e307).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2217/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r214253479
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/ArrayData.scala ---
@@ -34,6 +36,32 @@ object ArrayData {
case a: Array[Double] => UnsafeArrayData.fromPrimitiveArray(a)
case other => new GenericArrayData(other)
}
+
+
+ /**
+ * Allocate [[UnsafeArrayData]] or [[GenericArrayData]] based on given parameters.
+ *
+ * @param elementSize a size of an element in bytes
+ * @param numElements the number of elements the array should contain
+ * @param isPrimitiveType whether the type of an element is primitive type
+ * @param additionalErrorMessage string to include in the error message
+ */
+ def allocateArrayData(
+ elementSize: Int,
--- End diff --
`elementSize` is only used when creating unsafe array. I think we just have a `elementSize: Option[Int]` and remove the `isPrimitiveType` parameter.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95046/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r209858514
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/ArrayData.scala ---
@@ -34,6 +36,37 @@ object ArrayData {
case a: Array[Double] => UnsafeArrayData.fromPrimitiveArray(a)
case other => new GenericArrayData(other)
}
+
+
+ /**
+ * Allocate [[UnsafeArrayData]] or [[GenericArrayData]] based on given parameters.
+ *
+ * @param elementSize a size of an element in bytes
+ * @param numElements the number of elements the array should contain
+ * @param isPrimitiveType whether the type of an element is primitive type
+ * @param additionalErrorMessage string to include in the error message
+ */
+ def allocateArrayData(
+ elementSize: Int,
+ numElements : Long,
+ isPrimitiveType: Boolean,
+ additionalErrorMessage: String) : ArrayData = {
+ val arraySize = UnsafeArrayData.calculateSizeOfUnderlyingByteArray(numElements, elementSize)
+ if (isPrimitiveType && !UnsafeArrayData.shouldUseGenericArrayData(elementSize, numElements)) {
--- End diff --
The method `UnsafeArrayData.shouldUseGenericArrayData()` should be `UnsafeArrayData.shouldUseUnsafeArrayData()`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/94938/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #94421 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94421/testReport)** for PR 21912 at commit [`409cdd8`](https://github.com/apache/spark/commit/409cdd8667a423032e90b28f7b0e1487c332b60f).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #95487 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95487/testReport)** for PR 21912 at commit [`89b539e`](https://github.com/apache/spark/commit/89b539e2bd6ca28bff83a027e5dc41e91e87c6e4).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r213540440
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala ---
@@ -1314,9 +1314,9 @@ class CollectionExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper
}
test("Array Distinct") {
- val a0 = Literal.create(Seq(2, 1, 2, 3, 4, 4, 5), ArrayType(IntegerType))
+ val a0 = Literal.create(Seq(2, 1, 2, 3, 4, 4, 5), ArrayType(IntegerType, false))
--- End diff --
why this change?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2394/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [WIP][SPARK-24962][SQL] Refactor CodeGenerator.createUns...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #93753 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/93753/testReport)** for PR 21912 at commit [`043aa9d`](https://github.com/apache/spark/commit/043aa9d4a587ad904ab2e3af740dd209fbb3ac8a).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r210533929
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala ---
@@ -385,53 +385,81 @@ case class MapEntries(child: Expression) extends UnaryExpression with ExpectsInp
override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
nullSafeCodeGen(ctx, ev, c => {
+ val arrayData = ctx.freshName("arrayData")
val numElements = ctx.freshName("numElements")
val keys = ctx.freshName("keys")
val values = ctx.freshName("values")
val isKeyPrimitive = CodeGenerator.isPrimitiveType(childDataType.keyType)
val isValuePrimitive = CodeGenerator.isPrimitiveType(childDataType.valueType)
+
+ val wordSize = UnsafeRow.WORD_SIZE
+ val structSize = UnsafeRow.calculateBitSetWidthInBytes(2) + wordSize * 2
+ val elementSize = if (isKeyPrimitive && isValuePrimitive) {
+ Some(structSize + wordSize)
+ } else {
+ None
+ }
+
+ val allocation = ctx.createArrayData(arrayData, childDataType.keyType, numElements,
+ s" $prettyName failed.", elementSize = elementSize)
+ val setFunc = ctx.setArrayElementFunc(childDataType.keyType)
+
val code = if (isKeyPrimitive && isValuePrimitive) {
- genCodeForPrimitiveElements(ctx, keys, values, ev.value, numElements)
+ val genCodeForPrimitive = genCodeForPrimitiveElements(
+ ctx, arrayData, keys, values, ev.value, numElements, structSize, setFunc)
+ s"""
+ |if ($arrayData instanceof UnsafeArrayData) {
+ | $genCodeForPrimitive
+ |} else {
+ | ${genCodeForAnyElements(ctx, arrayData, keys, values, ev.value, numElements)}
+ |}
+ """.stripMargin
} else {
- genCodeForAnyElements(ctx, keys, values, ev.value, numElements)
+ s"${genCodeForAnyElements(ctx, arrayData, keys, values, ev.value, numElements)}"
}
+
s"""
|final int $numElements = $c.numElements();
|final ArrayData $keys = $c.keyArray();
|final ArrayData $values = $c.valueArray();
+ |$allocation
|$code
""".stripMargin
})
}
- private def getKey(varName: String) = CodeGenerator.getValue(varName, childDataType.keyType, "z")
+ private def getKey(varName: String, index: String) =
+ CodeGenerator.getValue(varName, childDataType.keyType, index)
- private def getValue(varName: String) = {
- CodeGenerator.getValue(varName, childDataType.valueType, "z")
- }
+ private def getValue(varName: String, index: String) =
+ CodeGenerator.getValue(varName, childDataType.valueType, index)
private def genCodeForPrimitiveElements(
ctx: CodegenContext,
+ arrayData: String,
keys: String,
values: String,
- arrayData: String,
- numElements: String): String = {
- val unsafeRow = ctx.freshName("unsafeRow")
+ resultArrayData: String,
+ numElements: String,
+ structSize: Int,
+ setFuncForKey: String): String = {
val unsafeArrayData = ctx.freshName("unsafeArrayData")
+ val baseObject = ctx.freshName("baseObject")
+ val unsafeRow = ctx.freshName("unsafeRow")
val structsOffset = ctx.freshName("structsOffset")
+ val offset = ctx.freshName("offset")
+ val z = ctx.freshName("z")
val calculateHeader = "UnsafeArrayData.calculateHeaderPortionInBytes"
val baseOffset = Platform.BYTE_ARRAY_OFFSET
val wordSize = UnsafeRow.WORD_SIZE
- val structSize = UnsafeRow.calculateBitSetWidthInBytes(2) + wordSize * 2
- val structSizeAsLong = structSize + "L"
- val keyTypeName = CodeGenerator.primitiveTypeName(childDataType.keyType)
+ val structSizeAsLong = s"${structSize}L"
val valueTypeName = CodeGenerator.primitiveTypeName(childDataType.valueType)
- val valueAssignment = s"$unsafeRow.set$valueTypeName(1, ${getValue(values)});"
+ val valueAssignment = s"$unsafeRow.set$valueTypeName(1, ${getValue(values, z)});"
--- End diff --
We should use `ctx.setArrayElementFunc` to get the func name?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2136/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [WIP][SPARK-24962][SQL] Refactor CodeGenerator.createUns...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/93753/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r210529856
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/ArrayData.scala ---
@@ -34,6 +36,32 @@ object ArrayData {
case a: Array[Double] => UnsafeArrayData.fromPrimitiveArray(a)
case other => new GenericArrayData(other)
}
+
+
+ /**
+ * Allocate [[UnsafeArrayData]] or [[GenericArrayData]] based on given parameters.
+ *
+ * @param elementSize a size of an element in bytes
+ * @param numElements the number of elements the array should contain
+ * @param isPrimitiveType whether the type of an element is primitive type
+ * @param additionalErrorMessage string to include in the error message
+ */
+ def allocateArrayData(
+ elementSize: Int,
+ numElements : Long,
+ isPrimitiveType: Boolean,
+ additionalErrorMessage: String) : ArrayData = {
+ if (isPrimitiveType && !UnsafeArrayData.shouldUseGenericArrayData(elementSize, numElements)) {
+ UnsafeArrayData.forPrimitiveArray(Platform.BYTE_ARRAY_OFFSET, numElements.toInt, elementSize)
--- End diff --
Do we need `Platform.BYTE_ARRAY_OFFSET` here? Seems like the method doesn't use the `offset` argument?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2746/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r214253006
--- Diff: sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/UnsafeArrayData.java ---
@@ -452,6 +452,16 @@ public UnsafeArrayData copy() {
public static UnsafeArrayData fromPrimitiveArray(
Object arr, int offset, int length, int elementSize) {
+ UnsafeArrayData result = createFreshArray(length, elementSize);
+ final long headerInBytes = calculateHeaderPortionInBytes(length);
+ final long valueRegionInBytes = (long)elementSize * length;
+ final Object data = result.getBaseObject();
--- End diff --
now the data is `Object` instead of `long[]`. Can we duplicate the code for now and think of how to deduplicate them later?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/21912
cc @ueshin @cloud-fan
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #94810 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94810/testReport)** for PR 21912 at commit [`b899259`](https://github.com/apache/spark/commit/b899259125ed2c7b94c0b121d6cbdeb4ee90285a).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #94691 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94691/testReport)** for PR 21912 at commit [`02aa518`](https://github.com/apache/spark/commit/02aa518428e06c9284879e3324636b4cd028246f).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r209643599
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -735,70 +735,100 @@ class CodegenContext {
}
/**
- * Generates code creating a [[UnsafeArrayData]].
+ * Generates code creating a [[UnsafeArrayData]] or [[GenericArrayData]] based on
+ * given parameters.
*
* @param arrayName name of the array to create
+ * @param elementType data type of the elements in source array
* @param numElements code representing the number of elements the array should contain
- * @param elementType data type of the elements in the array
* @param additionalErrorMessage string to include in the error message
+ * @param elementSize optional value which shows the size of an element of the allocated
+ * [[UnsafeArrayData]] or [[GenericArrayData]]
+ *
+ * @return code representing the allocation of [[ArrayData]]
+ * code representing a setter of an assignment for the generated array
*/
- def createUnsafeArray(
+ def createArrayData(
arrayName: String,
- numElements: String,
elementType: DataType,
- additionalErrorMessage: String): String = {
- val arraySize = freshName("size")
- val arrayBytes = freshName("arrayBytes")
+ numElements: String,
+ additionalErrorMessage: String,
+ elementSize: Option[Int] = None): (String, String) = {
--- End diff --
Yeah, they are exclusive. However, when `elementType` is not used, the integer value is required.
`MapEntries` stores `UnsafeRow` instead of primitive data. [This](https://github.com/apache/spark/blob/c15630014c7ef850fa21c1247fa5edd2b1bac81b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala#L405) requires non-fixed size unlike `elementType.defaultSize`.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #94653 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94653/testReport)** for PR 21912 at commit [`1a45a16`](https://github.com/apache/spark/commit/1a45a16b9a22d5accb83d00ece704b5eaf4e96c5).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [WIP][SPARK-24962][SQL] Refactor CodeGenerator.createUns...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r209856692
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -735,70 +735,98 @@ class CodegenContext {
}
/**
- * Generates code creating a [[UnsafeArrayData]].
+ * Generates code creating a [[UnsafeArrayData]] or [[GenericArrayData]] based on
+ * given parameters.
*
* @param arrayName name of the array to create
+ * @param elementType data type of the elements in source array
* @param numElements code representing the number of elements the array should contain
- * @param elementType data type of the elements in the array
* @param additionalErrorMessage string to include in the error message
+ * @param elementSize optional value which shows the size of an element of the allocated
+ * [[UnsafeArrayData]] or [[GenericArrayData]]
+ *
+ * @return code representing the allocation of [[ArrayData]]
+ * code representing a setter of an assignment for the generated array
*/
- def createUnsafeArray(
+ def createArrayData(
arrayName: String,
- numElements: String,
elementType: DataType,
- additionalErrorMessage: String): String = {
- val arraySize = freshName("size")
- val arrayBytes = freshName("arrayBytes")
+ numElements: String,
+ additionalErrorMessage: String,
+ elementSize: Option[Int] = None): (String, String) = {
+ val isPrimitiveType = if (elementSize.isDefined) {
+ false
+ } else {
+ CodeGenerator.isPrimitiveType(elementType)
+ }
- s"""
- |long $arraySize = UnsafeArrayData.calculateSizeOfUnderlyingByteArray(
- | $numElements,
- | ${elementType.defaultSize});
- |if ($arraySize > ${ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH}) {
- | throw new RuntimeException("Unsuccessful try create array with " + $arraySize +
- | " bytes of data due to exceeding the limit " +
- | "${ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH} bytes for UnsafeArrayData." +
- | "$additionalErrorMessage");
- |}
- |byte[] $arrayBytes = new byte[(int)$arraySize];
- |UnsafeArrayData $arrayName = new UnsafeArrayData();
- |Platform.putLong($arrayBytes, ${Platform.BYTE_ARRAY_OFFSET}, $numElements);
- |$arrayName.pointTo($arrayBytes, ${Platform.BYTE_ARRAY_OFFSET}, (int)$arraySize);
- """.stripMargin
+ val setFunc = if (isPrimitiveType) {
+ s"set${CodeGenerator.primitiveTypeName(elementType)}"
+ } else {
+ "update"
+ }
+
+ val elemSize = if (elementSize.isDefined) {
+ elementSize.get
+ } else {
+ elementType.defaultSize
+ }
+ val allocation =
+ s"""
+ |ArrayData $arrayName = ArrayData.allocateArrayData(
+ | $elemSize, $numElements, $isPrimitiveType, "$additionalErrorMessage");
+ """.stripMargin
+
+ (allocation, setFunc)
}
/**
- * Generates code creating a [[UnsafeArrayData]]. The generated code executes
- * a provided fallback when the size of backing array would exceed the array size limit.
- * @param arrayName a name of the array to create
- * @param numElements a piece of code representing the number of elements the array should contain
- * @param elementSize a size of an element in bytes
- * @param bodyCode a function generating code that fills up the [[UnsafeArrayData]]
- * and getting the backing array as a parameter
- * @param fallbackCode a piece of code executed when the array size limit is exceeded
+ * Generates assignment code for a [[ArrayData]]
+ *
+ * @param arrayName name of the array to create
+ * @param dataType data type of the result array
+ * @param elementType data type of the elements in source array
+ * @param srcArray code representing the number of elements the array should contain
+ * @param setFunc string to include in the error message
+ * @param rhsValue optional expression for the right-hand side of the returning assignment
+ * @param checkForNull optional value which shows whether a nullcheck is required for
+ * the returning assignment
+ *
+ * @return code representing an assignment to each element of the [[ArrayData]], which requires
+ * a pair of destination and source loop index variables
*/
- def createUnsafeArrayWithFallback(
+ def createArrayAssignment(
arrayName: String,
- numElements: String,
- elementSize: Int,
- bodyCode: String => String,
- fallbackCode: String): String = {
- val arraySize = freshName("size")
- val arrayBytes = freshName("arrayBytes")
- s"""
- |final long $arraySize = UnsafeArrayData.calculateSizeOfUnderlyingByteArray(
- | $numElements,
- | $elementSize);
- |if ($arraySize > ${ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH}) {
- | $fallbackCode
- |} else {
- | final byte[] $arrayBytes = new byte[(int)$arraySize];
- | UnsafeArrayData $arrayName = new UnsafeArrayData();
- | Platform.putLong($arrayBytes, ${Platform.BYTE_ARRAY_OFFSET}, $numElements);
- | $arrayName.pointTo($arrayBytes, ${Platform.BYTE_ARRAY_OFFSET}, (int)$arraySize);
- | ${bodyCode(arrayBytes)}
- |}
- """.stripMargin
+ dataType: DataType,
--- End diff --
How about passing `containsNull: Boolean` instead? Seems like we use only `dataType.asInstanceOf[ArrayType].containsNull` in this method.
Or `ArrayType` instead of both `elementType` and `containsNull`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #94812 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94812/testReport)** for PR 21912 at commit [`240936e`](https://github.com/apache/spark/commit/240936e98c6b32855236c7c635bacc0fb04acb76).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2309/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/94691/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2519/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2100/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2707/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r213539798
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -1490,6 +1423,63 @@ object CodeGenerator extends Logging {
}
}
+ /**
+ * Generates code creating a [[UnsafeArrayData]] or [[GenericArrayData]] based on
+ * given parameters.
+ *
+ * @param arrayName name of the array to create
+ * @param elementType data type of the elements in source array
+ * @param numElements code representing the number of elements the array should contain
+ * @param additionalErrorMessage string to include in the error message
+ * @param elementSize optional value which shows the size of an element of the allocated
--- End diff --
it's better to give an example when we will set it.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r213541643
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala ---
@@ -385,107 +385,120 @@ case class MapEntries(child: Expression) extends UnaryExpression with ExpectsInp
override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
nullSafeCodeGen(ctx, ev, c => {
+ val arrayData = ctx.freshName("arrayData")
val numElements = ctx.freshName("numElements")
val keys = ctx.freshName("keys")
val values = ctx.freshName("values")
val isKeyPrimitive = CodeGenerator.isPrimitiveType(childDataType.keyType)
val isValuePrimitive = CodeGenerator.isPrimitiveType(childDataType.valueType)
+
+ val wordSize = UnsafeRow.WORD_SIZE
+ val structSize = UnsafeRow.calculateBitSetWidthInBytes(2) + wordSize * 2
+ val elementSize = if (isKeyPrimitive && isValuePrimitive) {
+ Some(structSize + wordSize)
+ } else {
+ None
+ }
+
+ val allocation = CodeGenerator.createArrayData(arrayData, childDataType.keyType, numElements,
+ s" $prettyName failed.", elementSize = elementSize)
+
val code = if (isKeyPrimitive && isValuePrimitive) {
- genCodeForPrimitiveElements(ctx, keys, values, ev.value, numElements)
+ val genCodeForPrimitive = genCodeForPrimitiveElements(
+ ctx, arrayData, keys, values, ev.value, numElements, structSize)
+ s"""
+ |if ($arrayData instanceof UnsafeArrayData) {
+ | $genCodeForPrimitive
+ |} else {
+ | ${genCodeForAnyElements(ctx, arrayData, keys, values, ev.value, numElements)}
+ |}
+ """.stripMargin
} else {
- genCodeForAnyElements(ctx, keys, values, ev.value, numElements)
+ s"${genCodeForAnyElements(ctx, arrayData, keys, values, ev.value, numElements)}"
}
+
s"""
|final int $numElements = $c.numElements();
|final ArrayData $keys = $c.keyArray();
|final ArrayData $values = $c.valueArray();
+ |$allocation
|$code
""".stripMargin
})
}
- private def getKey(varName: String) = CodeGenerator.getValue(varName, childDataType.keyType, "z")
+ private def getKey(varName: String, index: String) =
+ CodeGenerator.getValue(varName, childDataType.keyType, index)
- private def getValue(varName: String) = {
- CodeGenerator.getValue(varName, childDataType.valueType, "z")
- }
+ private def getValue(varName: String, index: String) =
+ CodeGenerator.getValue(varName, childDataType.valueType, index)
private def genCodeForPrimitiveElements(
ctx: CodegenContext,
+ arrayData: String,
keys: String,
values: String,
- arrayData: String,
- numElements: String): String = {
- val unsafeRow = ctx.freshName("unsafeRow")
+ resultArrayData: String,
+ numElements: String,
+ structSize: Int): String = {
val unsafeArrayData = ctx.freshName("unsafeArrayData")
+ val baseObject = ctx.freshName("baseObject")
+ val unsafeRow = ctx.freshName("unsafeRow")
val structsOffset = ctx.freshName("structsOffset")
+ val offset = ctx.freshName("offset")
+ val z = ctx.freshName("z")
val calculateHeader = "UnsafeArrayData.calculateHeaderPortionInBytes"
val baseOffset = Platform.BYTE_ARRAY_OFFSET
val wordSize = UnsafeRow.WORD_SIZE
- val structSize = UnsafeRow.calculateBitSetWidthInBytes(2) + wordSize * 2
- val structSizeAsLong = structSize + "L"
- val keyTypeName = CodeGenerator.primitiveTypeName(childDataType.keyType)
- val valueTypeName = CodeGenerator.primitiveTypeName(childDataType.valueType)
-
- val valueAssignment = s"$unsafeRow.set$valueTypeName(1, ${getValue(values)});"
- val valueAssignmentChecked = if (childDataType.valueContainsNull) {
- s"""
- |if ($values.isNullAt(z)) {
- | $unsafeRow.setNullAt(1);
- |} else {
- | $valueAssignment
- |}
- """.stripMargin
- } else {
- valueAssignment
- }
+ val structSizeAsLong = s"${structSize}L"
- val assignmentLoop = (byteArray: String) =>
- s"""
- |final int $structsOffset = $calculateHeader($numElements) + $numElements * $wordSize;
- |UnsafeRow $unsafeRow = new UnsafeRow(2);
- |for (int z = 0; z < $numElements; z++) {
- | long offset = $structsOffset + z * $structSizeAsLong;
- | $unsafeArrayData.setLong(z, (offset << 32) + $structSizeAsLong);
- | $unsafeRow.pointTo($byteArray, $baseOffset + offset, $structSize);
- | $unsafeRow.set$keyTypeName(0, ${getKey(keys)});
- | $valueAssignmentChecked
- |}
- |$arrayData = $unsafeArrayData;
- """.stripMargin
+ val setKey =
+ CodeGenerator.setArrayElement(unsafeRow, childDataType.keyType, "0", getKey(keys, z))
--- End diff --
here we are setting value for row not array. Can we unify `setColumn` and `setArrayElement`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r214253824
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/ArrayData.scala ---
@@ -34,6 +36,32 @@ object ArrayData {
case a: Array[Double] => UnsafeArrayData.fromPrimitiveArray(a)
case other => new GenericArrayData(other)
}
+
+
+ /**
+ * Allocate [[UnsafeArrayData]] or [[GenericArrayData]] based on given parameters.
+ *
+ * @param elementSize a size of an element in bytes
+ * @param numElements the number of elements the array should contain
+ * @param isPrimitiveType whether the type of an element is primitive type
+ * @param additionalErrorMessage string to include in the error message
+ */
+ def allocateArrayData(
+ elementSize: Int,
--- End diff --
ah it's called in the generated code. Maybe we can use elementSize `-1` to create a generic array.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r209878827
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -735,70 +735,98 @@ class CodegenContext {
}
/**
- * Generates code creating a [[UnsafeArrayData]].
+ * Generates code creating a [[UnsafeArrayData]] or [[GenericArrayData]] based on
+ * given parameters.
*
* @param arrayName name of the array to create
+ * @param elementType data type of the elements in source array
* @param numElements code representing the number of elements the array should contain
- * @param elementType data type of the elements in the array
* @param additionalErrorMessage string to include in the error message
+ * @param elementSize optional value which shows the size of an element of the allocated
+ * [[UnsafeArrayData]] or [[GenericArrayData]]
+ *
+ * @return code representing the allocation of [[ArrayData]]
+ * code representing a setter of an assignment for the generated array
*/
- def createUnsafeArray(
+ def createArrayData(
--- End diff --
I think that it would be good to have a utility method to generate this call. WDYT?
On the other hand, I agree that we have a method `setArrayElementFunc` or `setArrayElement`. Thus, we can split this method into two methods that have only one return value.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #95199 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95199/testReport)** for PR 21912 at commit [`bfd8788`](https://github.com/apache/spark/commit/bfd878837fd1b8bb0ba29ba892f4d6ead9027e4d).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #95487 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95487/testReport)** for PR 21912 at commit [`89b539e`](https://github.com/apache/spark/commit/89b539e2bd6ca28bff83a027e5dc41e91e87c6e4).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [WIP][SPARK-24962][SQL] Refactor CodeGenerator.createUns...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/1940/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #94691 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94691/testReport)** for PR 21912 at commit [`02aa518`](https://github.com/apache/spark/commit/02aa518428e06c9284879e3324636b4cd028246f).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r209573854
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -735,70 +735,100 @@ class CodegenContext {
}
/**
- * Generates code creating a [[UnsafeArrayData]].
+ * Generates code creating a [[UnsafeArrayData]] or [[GenericArrayData]] based on
+ * given parameters.
*
* @param arrayName name of the array to create
+ * @param elementType data type of the elements in source array
* @param numElements code representing the number of elements the array should contain
- * @param elementType data type of the elements in the array
* @param additionalErrorMessage string to include in the error message
+ * @param elementSize optional value which shows the size of an element of the allocated
+ * [[UnsafeArrayData]] or [[GenericArrayData]]
+ *
+ * @return code representing the allocation of [[ArrayData]]
+ * code representing a setter of an assignment for the generated array
*/
- def createUnsafeArray(
+ def createArrayData(
arrayName: String,
- numElements: String,
elementType: DataType,
- additionalErrorMessage: String): String = {
- val arraySize = freshName("size")
- val arrayBytes = freshName("arrayBytes")
+ numElements: String,
+ additionalErrorMessage: String,
+ elementSize: Option[Int] = None): (String, String) = {
+ val isPrimitiveType = if (elementSize.isDefined) {
+ false
+ } else {
+ CodeGenerator.isPrimitiveType(elementType)
+ }
- s"""
- |long $arraySize = UnsafeArrayData.calculateSizeOfUnderlyingByteArray(
- | $numElements,
- | ${elementType.defaultSize});
- |if ($arraySize > ${ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH}) {
- | throw new RuntimeException("Unsuccessful try create array with " + $arraySize +
- | " bytes of data due to exceeding the limit " +
- | "${ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH} bytes for UnsafeArrayData." +
- | "$additionalErrorMessage");
- |}
- |byte[] $arrayBytes = new byte[(int)$arraySize];
- |UnsafeArrayData $arrayName = new UnsafeArrayData();
- |Platform.putLong($arrayBytes, ${Platform.BYTE_ARRAY_OFFSET}, $numElements);
- |$arrayName.pointTo($arrayBytes, ${Platform.BYTE_ARRAY_OFFSET}, (int)$arraySize);
- """.stripMargin
+ val setFunc = if (isPrimitiveType) {
+ s"set${CodeGenerator.primitiveTypeName(elementType)}"
+ } else {
+ "update"
+ }
+
+ val elemSize = if (elementSize.isDefined) {
+ elementSize.get
+ } else {
+ elementType.defaultSize
+ }
+ val arrayData = classOf[ArrayData].getName
+ val allocation =
+ s"""
+ |ArrayData $arrayName = $arrayData$$.MODULE$$.allocateArrayData(
--- End diff --
IIRC java can call methods of scala object directly, can you verify if `ArrayData.allocateArrayData` works?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r209573448
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -735,70 +735,100 @@ class CodegenContext {
}
/**
- * Generates code creating a [[UnsafeArrayData]].
+ * Generates code creating a [[UnsafeArrayData]] or [[GenericArrayData]] based on
+ * given parameters.
*
* @param arrayName name of the array to create
+ * @param elementType data type of the elements in source array
* @param numElements code representing the number of elements the array should contain
- * @param elementType data type of the elements in the array
* @param additionalErrorMessage string to include in the error message
+ * @param elementSize optional value which shows the size of an element of the allocated
+ * [[UnsafeArrayData]] or [[GenericArrayData]]
+ *
+ * @return code representing the allocation of [[ArrayData]]
+ * code representing a setter of an assignment for the generated array
*/
- def createUnsafeArray(
+ def createArrayData(
arrayName: String,
- numElements: String,
elementType: DataType,
- additionalErrorMessage: String): String = {
- val arraySize = freshName("size")
- val arrayBytes = freshName("arrayBytes")
+ numElements: String,
+ additionalErrorMessage: String,
+ elementSize: Option[Int] = None): (String, String) = {
--- End diff --
why do we need both `elementType` and `elementSize`.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #94646 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94646/testReport)** for PR 21912 at commit [`32dfcb5`](https://github.com/apache/spark/commit/32dfcb57b280884e57e464c3d718498510c9e307).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `trait ArraySetLike `
* `trait ArrayBinaryLike extends BinaryArrayExpressionWithImplicitCast with ArraySetLike `
* `case class ArrayUnion(left: Expression, right: Expression) extends ArrayBinaryLike`
* `case class ArrayIntersect(left: Expression, right: Expression) extends ArrayBinaryLike`
* `case class ArrayExcept(left: Expression, right: Expression) extends ArrayBinaryLike`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #94419 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94419/testReport)** for PR 21912 at commit [`5953668`](https://github.com/apache/spark/commit/5953668d58e216f31d1c3771e5ce116ae1332313).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/94684/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21912
**[Test build #95490 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95490/testReport)** for PR 21912 at commit [`61ea681`](https://github.com/apache/spark/commit/61ea68139125312749415dce65c23f797d54473d).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r210533651
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala ---
@@ -385,53 +385,81 @@ case class MapEntries(child: Expression) extends UnaryExpression with ExpectsInp
override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
nullSafeCodeGen(ctx, ev, c => {
+ val arrayData = ctx.freshName("arrayData")
val numElements = ctx.freshName("numElements")
val keys = ctx.freshName("keys")
val values = ctx.freshName("values")
val isKeyPrimitive = CodeGenerator.isPrimitiveType(childDataType.keyType)
val isValuePrimitive = CodeGenerator.isPrimitiveType(childDataType.valueType)
+
+ val wordSize = UnsafeRow.WORD_SIZE
+ val structSize = UnsafeRow.calculateBitSetWidthInBytes(2) + wordSize * 2
+ val elementSize = if (isKeyPrimitive && isValuePrimitive) {
+ Some(structSize + wordSize)
+ } else {
+ None
+ }
+
+ val allocation = ctx.createArrayData(arrayData, childDataType.keyType, numElements,
+ s" $prettyName failed.", elementSize = elementSize)
+ val setFunc = ctx.setArrayElementFunc(childDataType.keyType)
--- End diff --
We can delay this into `genCodeForPrimitiveElements`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r211126450
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -735,70 +735,98 @@ class CodegenContext {
}
/**
- * Generates code creating a [[UnsafeArrayData]].
+ * Generates code creating a [[UnsafeArrayData]] or [[GenericArrayData]] based on
+ * given parameters.
*
* @param arrayName name of the array to create
+ * @param elementType data type of the elements in source array
* @param numElements code representing the number of elements the array should contain
- * @param elementType data type of the elements in the array
* @param additionalErrorMessage string to include in the error message
+ * @param elementSize optional value which shows the size of an element of the allocated
+ * [[UnsafeArrayData]] or [[GenericArrayData]]
+ *
+ * @return code representing the allocation of [[ArrayData]]
+ * code representing a setter of an assignment for the generated array
*/
- def createUnsafeArray(
+ def createArrayData(
--- End diff --
I think it would be good to have this since this utility method will be used at other places where `new UnsafeArrayData` or `new GenericArrayData` is generated as Java code.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createUnsafeAr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21912
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/94812/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21912: [SPARK-24962][SQL] Refactor CodeGenerator.createU...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/21912#discussion_r211096868
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/ArrayData.scala ---
@@ -34,6 +36,32 @@ object ArrayData {
case a: Array[Double] => UnsafeArrayData.fromPrimitiveArray(a)
case other => new GenericArrayData(other)
}
+
+
+ /**
+ * Allocate [[UnsafeArrayData]] or [[GenericArrayData]] based on given parameters.
+ *
+ * @param elementSize a size of an element in bytes
+ * @param numElements the number of elements the array should contain
+ * @param isPrimitiveType whether the type of an element is primitive type
+ * @param additionalErrorMessage string to include in the error message
+ */
+ def allocateArrayData(
+ elementSize: Int,
+ numElements : Long,
+ isPrimitiveType: Boolean,
+ additionalErrorMessage: String) : ArrayData = {
+ if (isPrimitiveType && !UnsafeArrayData.shouldUseGenericArrayData(elementSize, numElements)) {
+ UnsafeArrayData.forPrimitiveArray(Platform.BYTE_ARRAY_OFFSET, numElements.toInt, elementSize)
--- End diff --
Good catch
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org