Skip to content
Closed
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 17 additions & 9 deletions mllib/src/main/scala/org/apache/spark/mllib/linalg/Matrices.scala
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ import com.github.fommil.netlib.BLAS.{getInstance => blas}
import org.apache.spark.annotation.Since
import org.apache.spark.ml.{linalg => newlinalg}
import org.apache.spark.sql.catalyst.InternalRow
import org.apache.spark.sql.catalyst.expressions.GenericMutableRow
import org.apache.spark.sql.catalyst.util.GenericArrayData
import org.apache.spark.sql.catalyst.expressions.{GenericMutableRow, UnsafeArrayData}
import org.apache.spark.sql.types._

/**
Expand Down Expand Up @@ -194,9 +193,9 @@ private[spark] class MatrixUDT extends UserDefinedType[Matrix] {
row.setByte(0, 0)
row.setInt(1, sm.numRows)
row.setInt(2, sm.numCols)
row.update(3, new GenericArrayData(sm.colPtrs.map(_.asInstanceOf[Any])))
row.update(4, new GenericArrayData(sm.rowIndices.map(_.asInstanceOf[Any])))
row.update(5, new GenericArrayData(sm.values.map(_.asInstanceOf[Any])))
row.update(3, UnsafeArrayData.fromPrimitiveArray(sm.colPtrs))
row.update(4, UnsafeArrayData.fromPrimitiveArray(sm.rowIndices))
row.update(5, UnsafeArrayData.fromPrimitiveArray(sm.values))
row.setBoolean(6, sm.isTransposed)

case dm: DenseMatrix =>
Expand All @@ -205,7 +204,7 @@ private[spark] class MatrixUDT extends UserDefinedType[Matrix] {
row.setInt(2, dm.numCols)
row.setNullAt(3)
row.setNullAt(4)
row.update(5, new GenericArrayData(dm.values.map(_.asInstanceOf[Any])))
row.update(5, UnsafeArrayData.fromPrimitiveArray(dm.values))
row.setBoolean(6, dm.isTransposed)
}
row
Expand All @@ -219,12 +218,21 @@ private[spark] class MatrixUDT extends UserDefinedType[Matrix] {
val tpe = row.getByte(0)
val numRows = row.getInt(1)
val numCols = row.getInt(2)
val values = row.getArray(5).toDoubleArray()
val values = row.getArray(5) match {
case u: UnsafeArrayData => u.toDoubleArrayUnchecked
case a => a.toDoubleArray()
}
val isTransposed = row.getBoolean(6)
tpe match {
case 0 =>
val colPtrs = row.getArray(3).toIntArray()
val rowIndices = row.getArray(4).toIntArray()
val colPtrs = row.getArray(3) match {
case u: UnsafeArrayData => u.toIntArrayUnchecked
case a => a.toIntArray()
}
val rowIndices = row.getArray(4) match {
case u: UnsafeArrayData => u.toIntArrayUnchecked
case a => a.toIntArray()
}
new SparseMatrix(numRows, numCols, colPtrs, rowIndices, values, isTransposed)
case 1 =>
new DenseMatrix(numRows, numCols, values, isTransposed)
Expand Down
24 changes: 16 additions & 8 deletions mllib/src/main/scala/org/apache/spark/mllib/linalg/Vectors.scala
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ import org.apache.spark.annotation.{AlphaComponent, Since}
import org.apache.spark.ml.{linalg => newlinalg}
import org.apache.spark.mllib.util.NumericParser
import org.apache.spark.sql.catalyst.InternalRow
import org.apache.spark.sql.catalyst.expressions.GenericMutableRow
import org.apache.spark.sql.catalyst.util.GenericArrayData
import org.apache.spark.sql.catalyst.expressions.{GenericMutableRow, UnsafeArrayData}
import org.apache.spark.sql.types._

/**
Expand Down Expand Up @@ -216,15 +215,15 @@ class VectorUDT extends UserDefinedType[Vector] {
val row = new GenericMutableRow(4)
row.setByte(0, 0)
row.setInt(1, size)
row.update(2, new GenericArrayData(indices.map(_.asInstanceOf[Any])))
row.update(3, new GenericArrayData(values.map(_.asInstanceOf[Any])))
row.update(2, UnsafeArrayData.fromPrimitiveArray(indices))
row.update(3, UnsafeArrayData.fromPrimitiveArray(values))
row
case DenseVector(values) =>
val row = new GenericMutableRow(4)
row.setByte(0, 1)
row.setNullAt(1)
row.setNullAt(2)
row.update(3, new GenericArrayData(values.map(_.asInstanceOf[Any])))
row.update(3, UnsafeArrayData.fromPrimitiveArray(values))
row
}
}
Expand All @@ -238,11 +237,20 @@ class VectorUDT extends UserDefinedType[Vector] {
tpe match {
case 0 =>
val size = row.getInt(1)
val indices = row.getArray(2).toIntArray()
val values = row.getArray(3).toDoubleArray()
val indices = row.getArray(2) match {
case u: UnsafeArrayData => u.toIntArrayUnchecked
case a => a.toIntArray()
}
val values = row.getArray(3) match {
case u: UnsafeArrayData => u.toDoubleArrayUnchecked
case a => a.toDoubleArray()
}
new SparseVector(size, indices, values)
case 1 =>
val values = row.getArray(3).toDoubleArray()
val values = row.getArray(3) match {
case u: UnsafeArrayData => u.toDoubleArrayUnchecked
case a => a.toDoubleArray()
}
new DenseVector(values)
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.spark.mllib.linalg

import org.apache.spark.sql.catalyst.encoders.ExpressionEncoder
import org.apache.spark.util.Benchmark

/**
* Serialization benchmark for VectorUDT.
*/
object UDTSerializationBenchmark {

def main(args: Array[String]): Unit = {
val iters = 1e2.toInt
val numRows = 1e3.toInt

val encoder = ExpressionEncoder[Vector].defaultBinding

val vectors = (1 to numRows).map { i =>
Vectors.dense(Array.fill(1e5.toInt)(1.0 * i))
}.toArray
val rows = vectors.map(encoder.toRow)

val benchmark = new Benchmark("VectorUDT de/serialization", numRows, iters)

benchmark.addCase("serialize") { _ =>
var sum = 0
var i = 0
while (i < numRows) {
sum += encoder.toRow(vectors(i)).numFields
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we call VectorUDT.serialize directly instead of encoder.toRows?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's different, VectorUDT.serialize only turn user object to catalyst data, but the real serialization should also include convert catalyst data into unsafe format.

i += 1
}
}

benchmark.addCase("deserialize") { _ =>
var sum = 0
var i = 0
while (i < numRows) {
sum += encoder.fromRow(rows(i)).numActives
i += 1
}
}

/*
Java HotSpot(TM) 64-Bit Server VM 1.8.0_60-b27 on Mac OS X 10.11.4
Intel(R) Core(TM) i7-4960HQ CPU @ 2.60GHz
VectorUDT de/serialization: Best/Avg Time(ms) Rate(M/s) Per Row(ns) Relative
-------------------------------------------------------------------------------------------
serialize 380 / 392 0.0 379730.0 1.0X
deserialize 138 / 142 0.0 137816.6 2.8X
*/
benchmark.run()
Copy link
Contributor Author

@cloud-fan cloud-fan Apr 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

result on master:

VectorUDT de/serialization:         Best/Avg Time(ms)    Rate(M/s)   Per Row(ns)   Relative
-------------------------------------------------------------------------------------------
serialize                                1414 / 1462          0.0     1414104.1       1.0X
deserialize                               169 /  178          0.0      169323.7       8.4X

The serialize is much faster now, but the deserialize isn't , investigating

Copy link
Contributor Author

@cloud-fan cloud-fan Apr 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did a micro benchmark, the toDoubleArray and the new toDoubleArrayUnchecked don't have much difference(the new one is only 20% faster). Maybe JVM can optimize simple while loop?

 def toDoubleArray(): Array[Double] = {
    val size = numElements()
    val values = new Array[Double](size)
    var i = 0
    while (i < size) {
      values(i) = getDouble(i)
      i += 1
    }
    values
  }

cc @davies

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, could you run the benchmark with more iterations to make sure that the C2 compiler could kick in (especially in Java 8)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I rerun the benchmark with 5 times higher iterations, but the result shows no difference.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we ran the test multiple times, and pick the best one, so that's fine.

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
* Instances of `UnsafeArrayData` act as pointers to row data stored in this format.
*/
// todo: there is a lof of duplicated code between UnsafeRow and UnsafeArrayData.
public class UnsafeArrayData extends ArrayData {
public final class UnsafeArrayData extends ArrayData {

private Object baseObject;
private long baseOffset;
Expand Down Expand Up @@ -81,7 +81,7 @@ private void assertIndexIsValid(int ordinal) {
}

public Object[] array() {
throw new UnsupportedOperationException("Only supported on GenericArrayData.");
throw new UnsupportedOperationException("Not supported on UnsafeArrayData.");
}

/**
Expand Down Expand Up @@ -336,4 +336,78 @@ public UnsafeArrayData copy() {
arrayCopy.pointTo(arrayDataCopy, Platform.BYTE_ARRAY_OFFSET, sizeInBytes);
return arrayCopy;
}

/**
* A faster version of `toIntArray`, which use memory copy instead of iterating all elements.
* Note that, this method is dangerous if this array contains null elements. We don't write
* null elements into the data region and memory copy will crash as the data size doesn't match.
*/
public int[] toIntArrayUnchecked() {
int[] result = new int[numElements];
Platform.copyMemory(baseObject, baseOffset + 4 + 4L * numElements,
result, Platform.INT_ARRAY_OFFSET, 4L * numElements);
return result;
}

/**
* A faster version of `toDoubleArray`, which use memory copy instead of iterating all elements.
* Note that, this method is dangerous if this array contains null elements. We don't write
* null elements into the data region and memory copy will crash as the data size doesn't match.
*/
public double[] toDoubleArrayUnchecked() {
double[] result = new double[numElements];
Platform.copyMemory(baseObject, baseOffset + 4 + 4L * numElements,
result, Platform.DOUBLE_ARRAY_OFFSET, 8L * numElements);
return result;
}

public static UnsafeArrayData fromPrimitiveArray(int[] arr) {
int offsetRegionSize = 4 * arr.length;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check that 4 * arr.length won't overflow.

int valueRegionSize = 4 * arr.length;
int totalSize = 4 + offsetRegionSize + valueRegionSize;
byte[] data = new byte[totalSize];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not an unsafe array. Should we allocate a direct buffer instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it doesn't matter, unsafe row also uses on-heap byte array

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mengxr We only use off-heap memory (direct buffer) for page (tens of MB), otherwise always use on-heap arrays. Off heap memory may be less efficient to handle smaller allocates.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. We might have very large vectors, e.g., 10 million values. But it is not very common.

@cloud-fan Can we use long[] or double[] to back up the buffer? So we can store more elements. Right now the upper bound is about 3e8, which might be sufficient. But if a simple change would increase the limit, that would be better..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After think about it, we can't support very long array in UnsafeArrayData, as the offset region only use 4 bytes to encode the value offset. I'm going to add a validation so that we won't overflow if given a big array.

Actually we can improve the unsafe format for array data, which is similar to unsafe row:

[null bits] [values] [variable length portion]

One difference is that, the values region in unsafe row is 8-byte per field, but for array, it should depend on the data size, e.g. 1 byte for boolean. Then we can just memory copy primitive array into values region and no need to care about the offset region.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1


Platform.putInt(data, Platform.BYTE_ARRAY_OFFSET, arr.length);

int offsetPosition = Platform.BYTE_ARRAY_OFFSET + 4;
int valueOffset = 4 + offsetRegionSize;
for (int i = 0; i < arr.length; i++) {
Platform.putInt(data, offsetPosition, valueOffset);
offsetPosition += 4;
valueOffset += 4;
}

Platform.copyMemory(arr, Platform.INT_ARRAY_OFFSET, data,
Platform.BYTE_ARRAY_OFFSET + 4 + offsetRegionSize, valueRegionSize);

UnsafeArrayData result = new UnsafeArrayData();
result.pointTo(data, Platform.BYTE_ARRAY_OFFSET, totalSize);
return result;
}

public static UnsafeArrayData fromPrimitiveArray(double[] arr) {
int offsetRegionSize = 4 * arr.length;
int valueRegionSize = 8 * arr.length;
int totalSize = 4 + offsetRegionSize + valueRegionSize;
byte[] data = new byte[totalSize];

Platform.putInt(data, Platform.BYTE_ARRAY_OFFSET, arr.length);

int offsetPosition = Platform.BYTE_ARRAY_OFFSET + 4;
int valueOffset = 4 + offsetRegionSize;
for (int i = 0; i < arr.length; i++) {
Platform.putInt(data, offsetPosition, valueOffset);
offsetPosition += 4;
valueOffset += 8;
}

Platform.copyMemory(arr, Platform.DOUBLE_ARRAY_OFFSET, data,
Platform.BYTE_ARRAY_OFFSET + 4 + offsetRegionSize, valueRegionSize);

UnsafeArrayData result = new UnsafeArrayData();
result.pointTo(data, Platform.BYTE_ARRAY_OFFSET, totalSize);
return result;
}

// TODO: add more specialized methods.
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
* [unsafe key array numBytes] [unsafe key array] [unsafe value array]
*/
// TODO: Use a more efficient format which doesn't depend on unsafe array.
public class UnsafeMapData extends MapData {
public final class UnsafeMapData extends MapData {

private Object baseObject;
private long baseOffset;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.spark.sql.catalyst.util

import org.apache.spark.SparkFunSuite
import org.apache.spark.sql.catalyst.expressions.UnsafeArrayData

class UnsafeArraySuite extends SparkFunSuite {

test("from primitive int array") {
val array = Array(1, 10, 100)
val unsafe = UnsafeArrayData.fromPrimitiveArray(array)
assert(unsafe.numElements == 3)
assert(unsafe.getSizeInBytes == 4 + 4 * 3 + 4 * 3)
assert(unsafe.getInt(0) == 1)
assert(unsafe.getInt(1) == 10)
assert(unsafe.getInt(2) == 100)
}

test("from primitive double array") {
val array = Array(1.1, 2.2, 3.3)
val unsafe = UnsafeArrayData.fromPrimitiveArray(array)
assert(unsafe.numElements == 3)
assert(unsafe.getSizeInBytes == 4 + 4 * 3 + 8 * 3)
assert(unsafe.getDouble(0) == 1.1)
assert(unsafe.getDouble(1) == 2.2)
assert(unsafe.getDouble(2) == 3.3)
}

test("to int array unchecked") {
val array = Array(1, 10, 100)
val unsafe = UnsafeArrayData.fromPrimitiveArray(array)
val array2 = unsafe.toIntArrayUnchecked
assert(array.toSeq == array2.toSeq)
}

test("to double array unchecked") {
val array = Array(1.1, 2.2, 3.3)
val unsafe = UnsafeArrayData.fromPrimitiveArray(array)
val array2 = unsafe.toDoubleArrayUnchecked
assert(array.toSeq == array2.toSeq)
}
}