You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by dbtsai <gi...@git.apache.org> on 2014/11/16 02:08:32 UTC
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient active...
GitHub user dbtsai opened a pull request:
https://github.com/apache/spark/pull/3288
[SPARK-4431][MLlib] Implement efficient activeIterator for dense and sparse vector
Previously, we were using Breeze's activeIterator to access the non-zero elements
in sparse vector, and explicitly skipping the zero in dense/sparse vector using
pattern matching. Due to the overhead, we switched back to native `while loop`
in #SPARK-4129.
However, #SPARK-4129 requires de-reference the dv.values/sv.values in
each access to the value, and the zeros in dense vector and sparse vector
if exist are skipped in the add function call; the overall penalty will be
around 10% compared with de-reference once outside the while block,
and checking if zero before calling the add function. The code is branched out
for dense and sparse vector, and it's not easy to maintain in the long term.
Not only this activeIterator implementation increases the performance,
but the abstraction of accessing the non-zero elements in different
vector type also helps the maintainability of codebase. In this PR,
only MultivariateOnlineSummarizer uses new API as example,
and others can be migrated to activeIterator later.
Benchmarking with mnist8m dataset on single JVM
with first 200 samples loaded in memory, and repeating 5000 times.
Before change:
Sparse Vector - 30.02
Dense Vector - 38.27
After this optimization:
Sparse Vector - 27.54
Dense Vector - 35.13
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/AlpineNow/spark activeIterator
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/3288.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 #3288
----
commit 101c2eafb250b428f1b244e7f8057e63400f8f4e
Author: DB Tsai <db...@alpinenow.com>
Date: 2014-11-13T07:08:13Z
Finished SPARK-4431
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient active...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3288#issuecomment-63910641
[Test build #23694 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23694/consoleFull) for PR 3288 at commit [`1907ae1`](https://github.com/apache/spark/commit/1907ae122ac0f385e5c408b827bd438e209cd71e).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient active...
Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on a diff in the pull request:
https://github.com/apache/spark/pull/3288#discussion_r20553260
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/Vectors.scala ---
@@ -76,6 +76,22 @@ sealed trait Vector extends Serializable {
def copy: Vector = {
throw new NotImplementedError(s"copy is not implemented for ${this.getClass}.")
}
+
+ /**
+ * It will return the iterator for the active elements of dense and sparse vector as
+ * (index, value) pair. Note that foreach method can be overridden for better performance
+ * in different vector implementation.
+ *
+ * @param skippingZeros Skipping zero elements explicitly if true. It will be useful when we
+ * iterator through dense vector having lots of zero elements which
+ * we want to skip. Default is false.
+ * @return Iterator[(Int, Double)] where the first element in the tuple is the index,
+ * and the second element is the corresponding value.
+ */
+ private[spark] def activeIterator(skippingZeros: Boolean): Iterator[(Int, Double)]
--- End diff --
With the following code,
sample.activeIterator(false).foreach {
case (index, value) => if(value != 0.0) add(index, value)
}
It takes 61.809 for dense vector, and 54.626 for sparse vector.
The most expensive part is calling the anonymous function even when the values are zero.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient active...
Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on a diff in the pull request:
https://github.com/apache/spark/pull/3288#discussion_r20688070
--- Diff: mllib/src/test/scala/org/apache/spark/mllib/linalg/VectorsSuite.scala ---
@@ -173,4 +173,63 @@ class VectorsSuite extends FunSuite {
val v = Vectors.fromBreeze(x(::, 0))
assert(v.size === x.rows)
}
+
+ test("foreach") {
+ val dv = Vectors.dense(0.0, 1.2, 3.1, 0.0)
+ val sv = Vectors.sparse(4, Seq((1, 1.2), (2, 3.1), (3, 0.0)))
+
+ val dvMap0 = scala.collection.mutable.Map[Int, Double]()
+ dv.foreach() {
+ case (index: Int, value: Double) => dvMap0.put(index, value)
+ }
--- End diff --
Calling `foreach` without parenthesis
```scala
dv.foreach {
case (index: Int, value: Double) => dvMap0.put(index, value)
}
```
will cause
```
Error:(182, 16) missing parameter type for expanded function
The argument types of an anonymous function must be fully known. (SLS 8.5)
Expected type was: Boolean
dv.foreach {
^
```
This is scala curry function overloading issue. It seems that unless we change the signature to
```scala
private[spark] def foreach(skippingZeros: Boolean = false, f: ((Int, Double)) => Unit)
```
we need to explicitly call it with parenthesis when we want to call it with default value of skippingZeros.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient active...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3288#issuecomment-63266570
[Test build #23462 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23462/consoleFull) for PR 3288 at commit [`453b29f`](https://github.com/apache/spark/commit/453b29fbd57b2b7259d62aa554c9cb4548f4f802).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient active...
Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on a diff in the pull request:
https://github.com/apache/spark/pull/3288#discussion_r20549368
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/Vectors.scala ---
@@ -76,6 +76,22 @@ sealed trait Vector extends Serializable {
def copy: Vector = {
throw new NotImplementedError(s"copy is not implemented for ${this.getClass}.")
}
+
+ /**
+ * It will return the iterator for the active elements of dense and sparse vector as
+ * (index, value) pair. Note that foreach method can be overridden for better performance
+ * in different vector implementation.
+ *
+ * @param skippingZeros Skipping zero elements explicitly if true. It will be useful when we
+ * iterator through dense vector having lots of zero elements which
+ * we want to skip. Default is false.
+ * @return Iterator[(Int, Double)] where the first element in the tuple is the index,
+ * and the second element is the corresponding value.
+ */
+ private[spark] def activeIterator(skippingZeros: Boolean): Iterator[(Int, Double)]
--- End diff --
I think the following code should have the same performance:
~~~scala
vec.foreach { (i, v) =>
if (v != 0.0) {
...
}
}
~~~
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient active...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3288#issuecomment-63577206
[Test build #23568 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23568/consoleFull) for PR 3288 at commit [`d970498`](https://github.com/apache/spark/commit/d970498b58af8abbdd9879ba6055ca1bfbfd392c).
* This patch **passes all tests**.
* This patch **does not merge cleanly**.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient foreac...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3288#issuecomment-64052641
[Test build #23731 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23731/consoleFull) for PR 3288 at commit [`03dd693`](https://github.com/apache/spark/commit/03dd6938e853b1fcc5c4d26fb1decc375ce5ab8c).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient foreac...
Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on a diff in the pull request:
https://github.com/apache/spark/pull/3288#discussion_r20748806
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/stat/MultivariateOnlineSummarizer.scala ---
@@ -83,34 +58,37 @@ class MultivariateOnlineSummarizer extends MultivariateStatisticalSummary with S
require(sample.size > 0, s"Vector should have dimension larger than zero.")
n = sample.size
- currMean = BDV.zeros[Double](n)
- currM2n = BDV.zeros[Double](n)
- currM2 = BDV.zeros[Double](n)
- currL1 = BDV.zeros[Double](n)
- nnz = BDV.zeros[Double](n)
- currMax = BDV.fill(n)(Double.MinValue)
- currMin = BDV.fill(n)(Double.MaxValue)
+ currMean = Array.ofDim[Double](n)
+ currM2n = Array.ofDim[Double](n)
+ currM2 = Array.ofDim[Double](n)
+ currL1 = Array.ofDim[Double](n)
+ nnz = Array.ofDim[Double](n)
+ currMax = Array.fill[Double](n)(Double.MinValue)
+ currMin = Array.fill[Double](n)(Double.MaxValue)
}
require(n == sample.size, s"Dimensions mismatch when adding new sample." +
s" Expecting $n but got ${sample.size}.")
- sample match {
- case dv: DenseVector => {
- var j = 0
- while (j < dv.size) {
- add(j, dv.values(j))
- j += 1
+ sample.foreachActive((index, value) => {
+ if(value != 0.0){
--- End diff --
space after `if` and before `{`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient active...
Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on a diff in the pull request:
https://github.com/apache/spark/pull/3288#discussion_r20557238
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/Vectors.scala ---
@@ -76,6 +76,22 @@ sealed trait Vector extends Serializable {
def copy: Vector = {
throw new NotImplementedError(s"copy is not implemented for ${this.getClass}.")
}
+
+ /**
+ * It will return the iterator for the active elements of dense and sparse vector as
+ * (index, value) pair. Note that foreach method can be overridden for better performance
+ * in different vector implementation.
+ *
+ * @param skippingZeros Skipping zero elements explicitly if true. It will be useful when we
+ * iterator through dense vector having lots of zero elements which
+ * we want to skip. Default is false.
+ * @return Iterator[(Int, Double)] where the first element in the tuple is the index,
+ * and the second element is the corresponding value.
+ */
+ private[spark] def activeIterator(skippingZeros: Boolean): Iterator[(Int, Double)]
--- End diff --
`Tuple2[Int, Double]` is specialized in Scala: https://github.com/scala/scala/blob/2.10.x/src/library/scala/Tuple2.scala, but the iterator interface still won't be high-performance for numerical computation. The iterator interface is not used in this PR, as we only need `foreach`. Then the best for us is defining `foreach` directly:
~~~scala
def foreach(f: (Int, Double) => Unit)
~~~
( We could implement `ZippedTraversable2` but it doesn't seem to be necessary.)
or
~~~scala
def foreach(skipZeros = True)(f: (Int, Double) => Unit)
~~~
This is a private function. We can change it when we see more use cases.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient active...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3288#issuecomment-63566518
[Test build #23568 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23568/consoleFull) for PR 3288 at commit [`d970498`](https://github.com/apache/spark/commit/d970498b58af8abbdd9879ba6055ca1bfbfd392c).
* This patch **does not merge cleanly**.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient foreac...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3288#issuecomment-64059973
[Test build #23731 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23731/consoleFull) for PR 3288 at commit [`03dd693`](https://github.com/apache/spark/commit/03dd6938e853b1fcc5c4d26fb1decc375ce5ab8c).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient active...
Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on a diff in the pull request:
https://github.com/apache/spark/pull/3288#discussion_r20687461
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/stat/MultivariateOnlineSummarizer.scala ---
@@ -95,22 +93,7 @@ class MultivariateOnlineSummarizer extends MultivariateStatisticalSummary with S
require(n == sample.size, s"Dimensions mismatch when adding new sample." +
s" Expecting $n but got ${sample.size}.")
- sample match {
- case dv: DenseVector => {
- var j = 0
- while (j < dv.size) {
- add(j, dv.values(j))
- j += 1
- }
- }
- case sv: SparseVector =>
- var j = 0
- while (j < sv.indices.size) {
- add(sv.indices(j), sv.values(j))
- j += 1
- }
- case v => throw new IllegalArgumentException("Do not support vector type " + v.getClass)
- }
+ sample.foreach(true)(x => add(x._1, x._2))
--- End diff --
This will be faster than
```scala
sample.foreach(true){
case (index, value) => add(index, value)
}
```
for 5%.
See the generated bytecode.
With pattern matching.
```
L20
LINENUMBER 103 L20
ALOAD 4
INVOKEVIRTUAL scala/Tuple2._1$mcI$sp ()I
ISTORE 5
L21
ALOAD 4
INVOKEVIRTUAL scala/Tuple2._2$mcD$sp ()D
DSTORE 6
L22
ALOAD 0
ILOAD 5
DLOAD 6
INVOKEVIRTUAL org/apache/spark/mllib/stat/MultivariateOnlineSummarizer.org$apache$spark$mllib$stat$MultivariateOnlineSummarizer$$add (ID)V
GETSTATIC scala/runtime/BoxedUnit.UNIT : Lscala/runtime/BoxedUnit;
ASTORE 8
```
Without pattern matching.
```
L17
LINENUMBER 100 L17
ALOAD 0
ALOAD 3
INVOKEVIRTUAL scala/Tuple2._1$mcI$sp ()I
ALOAD 3
INVOKEVIRTUAL scala/Tuple2._2$mcD$sp ()D
INVOKEVIRTUAL org/apache/spark/mllib/stat/MultivariateOnlineSummarizer.org$apache$spark$mllib$stat$MultivariateOnlineSummarizer$$add (ID)V
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient active...
Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on the pull request:
https://github.com/apache/spark/pull/3288#issuecomment-63566328
(PS, when I did the bytecode analysis, I found that accessing the
member variables of values and values.size require two operation.
By having a local copy of reference to make it as single call, there is
another 8% performance gain. See
http://stackoverflow.com/questions/6602922/is-it-faster-to-access-final-local-variables-than-class-variables-in-java for detail)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient active...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3288#issuecomment-63201701
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23430/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient foreac...
Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai closed the pull request at:
https://github.com/apache/spark/pull/3288
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient foreac...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3288#issuecomment-64059980
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23731/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient active...
Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on a diff in the pull request:
https://github.com/apache/spark/pull/3288#discussion_r20473209
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/Vectors.scala ---
@@ -76,6 +76,22 @@ sealed trait Vector extends Serializable {
def copy: Vector = {
throw new NotImplementedError(s"copy is not implemented for ${this.getClass}.")
}
+
+ /**
+ * It will return the iterator for the active elements of dense and sparse vector as
+ * (index, value) pair. Note that foreach method can be overridden for better performance
+ * in different vector implementation.
+ *
+ * @param skippingZeros Skipping zero elements explicitly if true. It will be useful when we
+ * iterator through dense vector having lots of zero elements which
+ * we want to skip. Default is false.
+ * @return Iterator[(Int, Double)] where the first element in the tuple is the index,
+ * and the second element is the corresponding value.
+ */
+ private[spark] def activeIterator(skippingZeros: Boolean): Iterator[(Int, Double)]
--- End diff --
I think we don't need `skippingZeros` here because it is very easy to chain the iterator with a filter to achieve it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient active...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3288#issuecomment-63910649
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23694/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient foreac...
Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on a diff in the pull request:
https://github.com/apache/spark/pull/3288#discussion_r20748820
--- Diff: mllib/src/test/scala/org/apache/spark/mllib/linalg/VectorsSuite.scala ---
@@ -173,4 +173,24 @@ class VectorsSuite extends FunSuite {
val v = Vectors.fromBreeze(x(::, 0))
assert(v.size === x.rows)
}
+
+ test("foreach") {
--- End diff --
foreachActive
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient foreac...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3288#issuecomment-64060832
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23733/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient active...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3288#issuecomment-63199990
[Test build #23430 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23430/consoleFull) for PR 3288 at commit [`101c2ea`](https://github.com/apache/spark/commit/101c2eafb250b428f1b244e7f8057e63400f8f4e).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient active...
Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on a diff in the pull request:
https://github.com/apache/spark/pull/3288#discussion_r20533260
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/Vectors.scala ---
@@ -273,6 +289,47 @@ class DenseVector(val values: Array[Double]) extends Vector {
override def copy: DenseVector = {
new DenseVector(values.clone())
}
+
+ private[spark] override def activeIterator(skippingZeros: Boolean) = new Iterator[(Int, Double)] {
+ private var i = 0
+ private val valuesSize = values.size
+
+ // If zeros are asked to be explicitly skipped, the parent `size` method is called to count
+ // the number of nonzero elements using `hasNext` and `next` methods.
+ override lazy val size: Int = if (skippingZeros) super.size else valuesSize
+
+ override def hasNext = {
+ if (skippingZeros) {
+ var found = false
+ while (!found && i < valuesSize) if (values(i) != 0.0) found = true else i += 1
+ }
+ i < valuesSize
+ }
+
+ override def next = {
+ val result = (i, values(i))
+ i += 1
+ result
+ }
+
+ override def foreach[@specialized(Unit) U](f: ((Int, Double)) => U) {
--- End diff --
Interesting. In scala's range code, they have
@inline final override def foreach[@specialized(Unit) U](f: Int => U)
I'll do a bytecode analysis, and see if it will generate the same bytecode.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient active...
Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on a diff in the pull request:
https://github.com/apache/spark/pull/3288#discussion_r20473211
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/Vectors.scala ---
@@ -273,6 +289,47 @@ class DenseVector(val values: Array[Double]) extends Vector {
override def copy: DenseVector = {
new DenseVector(values.clone())
}
+
+ private[spark] override def activeIterator(skippingZeros: Boolean) = new Iterator[(Int, Double)] {
+ private var i = 0
+ private val valuesSize = values.size
+
+ // If zeros are asked to be explicitly skipped, the parent `size` method is called to count
+ // the number of nonzero elements using `hasNext` and `next` methods.
+ override lazy val size: Int = if (skippingZeros) super.size else valuesSize
+
+ override def hasNext = {
+ if (skippingZeros) {
+ var found = false
+ while (!found && i < valuesSize) if (values(i) != 0.0) found = true else i += 1
+ }
+ i < valuesSize
+ }
+
+ override def next = {
+ val result = (i, values(i))
+ i += 1
+ result
+ }
+
+ override def foreach[@specialized(Unit) U](f: ((Int, Double)) => U) {
--- End diff --
Another option is force the return type to be `Unit`. Scala compiler will know how to handle `f` with other return types.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient active...
Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on a diff in the pull request:
https://github.com/apache/spark/pull/3288#discussion_r20532934
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/Vectors.scala ---
@@ -76,6 +76,22 @@ sealed trait Vector extends Serializable {
def copy: Vector = {
throw new NotImplementedError(s"copy is not implemented for ${this.getClass}.")
}
+
+ /**
+ * It will return the iterator for the active elements of dense and sparse vector as
+ * (index, value) pair. Note that foreach method can be overridden for better performance
+ * in different vector implementation.
+ *
+ * @param skippingZeros Skipping zero elements explicitly if true. It will be useful when we
+ * iterator through dense vector having lots of zero elements which
+ * we want to skip. Default is false.
+ * @return Iterator[(Int, Double)] where the first element in the tuple is the index,
+ * and the second element is the corresponding value.
+ */
+ private[spark] def activeIterator(skippingZeros: Boolean): Iterator[(Int, Double)]
--- End diff --
`skippingZeros` will be very useful in `foreach` operation, and if you use iterator -> filter -> foreach, it will not use the optimized `foreach` which is implemented by native while loop.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient active...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3288#issuecomment-63577210
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23568/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient active...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3288#issuecomment-63201699
[Test build #23430 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23430/consoleFull) for PR 3288 at commit [`101c2ea`](https://github.com/apache/spark/commit/101c2eafb250b428f1b244e7f8057e63400f8f4e).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient active...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3288#issuecomment-63272286
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23462/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient active...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3288#issuecomment-63272284
[Test build #23462 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23462/consoleFull) for PR 3288 at commit [`453b29f`](https://github.com/apache/spark/commit/453b29fbd57b2b7259d62aa554c9cb4548f4f802).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient active...
Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on a diff in the pull request:
https://github.com/apache/spark/pull/3288#discussion_r20554090
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/Vectors.scala ---
@@ -76,6 +76,22 @@ sealed trait Vector extends Serializable {
def copy: Vector = {
throw new NotImplementedError(s"copy is not implemented for ${this.getClass}.")
}
+
+ /**
+ * It will return the iterator for the active elements of dense and sparse vector as
+ * (index, value) pair. Note that foreach method can be overridden for better performance
+ * in different vector implementation.
+ *
+ * @param skippingZeros Skipping zero elements explicitly if true. It will be useful when we
+ * iterator through dense vector having lots of zero elements which
+ * we want to skip. Default is false.
+ * @return Iterator[(Int, Double)] where the first element in the tuple is the index,
+ * and the second element is the corresponding value.
+ */
+ private[spark] def activeIterator(skippingZeros: Boolean): Iterator[(Int, Double)]
--- End diff --
Okay, the issue is in the anonymous function. Basically, scala will convert primitive index: Int and value: Double into boxed object in order to have them in tuple. In my testing dataset, there are so many zeros explicitly, and even those values with zero have to be converted to tuple before we do the `if statement`. That's why it's dramatically faster if we do the `if statement` before calling the anonymous function.
Changing the signature of `foreach` into
def foreach[@specialized(Unit) U](f: (Int, Double) => U)
to take two primitive variables will solve this problem, but it will not comply the interface of `foreach`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient foreac...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3288#issuecomment-64060829
[Test build #23733 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23733/consoleFull) for PR 3288 at commit [`844b0e6`](https://github.com/apache/spark/commit/844b0e6e8e022c90fa93ff2781bf77c907cf5bb7).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient foreac...
Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on the pull request:
https://github.com/apache/spark/pull/3288#issuecomment-64052698
LGTM except minor inline comments. Thank for improving the performance!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient foreac...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3288#issuecomment-64054024
[Test build #23733 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23733/consoleFull) for PR 3288 at commit [`844b0e6`](https://github.com/apache/spark/commit/844b0e6e8e022c90fa93ff2781bf77c907cf5bb7).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient foreac...
Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on the pull request:
https://github.com/apache/spark/pull/3288#issuecomment-64064680
Merged into master and branch-1.2. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient active...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3288#issuecomment-63901964
[Test build #23694 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23694/consoleFull) for PR 3288 at commit [`1907ae1`](https://github.com/apache/spark/commit/1907ae122ac0f385e5c408b827bd438e209cd71e).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient foreac...
Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on a diff in the pull request:
https://github.com/apache/spark/pull/3288#discussion_r20748804
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/stat/MultivariateOnlineSummarizer.scala ---
@@ -83,34 +58,37 @@ class MultivariateOnlineSummarizer extends MultivariateStatisticalSummary with S
require(sample.size > 0, s"Vector should have dimension larger than zero.")
n = sample.size
- currMean = BDV.zeros[Double](n)
- currM2n = BDV.zeros[Double](n)
- currM2 = BDV.zeros[Double](n)
- currL1 = BDV.zeros[Double](n)
- nnz = BDV.zeros[Double](n)
- currMax = BDV.fill(n)(Double.MinValue)
- currMin = BDV.fill(n)(Double.MaxValue)
+ currMean = Array.ofDim[Double](n)
+ currM2n = Array.ofDim[Double](n)
+ currM2 = Array.ofDim[Double](n)
+ currL1 = Array.ofDim[Double](n)
+ nnz = Array.ofDim[Double](n)
+ currMax = Array.fill[Double](n)(Double.MinValue)
+ currMin = Array.fill[Double](n)(Double.MaxValue)
}
require(n == sample.size, s"Dimensions mismatch when adding new sample." +
s" Expecting $n but got ${sample.size}.")
- sample match {
- case dv: DenseVector => {
- var j = 0
- while (j < dv.size) {
- add(j, dv.values(j))
- j += 1
+ sample.foreachActive((index, value) => {
--- End diff --
The following style is more common in the codebase:
~~~
sample.foreachActive { (index, value) =>
...
}
~~~
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient active...
Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on a diff in the pull request:
https://github.com/apache/spark/pull/3288#discussion_r20615000
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/Vectors.scala ---
@@ -76,6 +76,22 @@ sealed trait Vector extends Serializable {
def copy: Vector = {
throw new NotImplementedError(s"copy is not implemented for ${this.getClass}.")
}
+
+ /**
+ * It will return the iterator for the active elements of dense and sparse vector as
+ * (index, value) pair. Note that foreach method can be overridden for better performance
+ * in different vector implementation.
+ *
+ * @param skippingZeros Skipping zero elements explicitly if true. It will be useful when we
+ * iterator through dense vector having lots of zero elements which
+ * we want to skip. Default is false.
+ * @return Iterator[(Int, Double)] where the first element in the tuple is the index,
+ * and the second element is the corresponding value.
+ */
+ private[spark] def activeIterator(skippingZeros: Boolean): Iterator[(Int, Double)]
--- End diff --
You are right; the `Tuple2[Int, Double]` is specialized, and I mistakenly interpreted the bytecode.
For the flowing scala code,
```scala
def foreach[@specialized(Unit) U](f: ((Int, Double)) => U) {
var i = 0
val localValuesSize = values.size
val localValues = values
while (i < localValuesSize) {
f(i, localValues(i))
i += 1
}
}
```
the generated bytecode will be
```
public foreach(Lscala/Function1;)V
L0
LINENUMBER 296 L0
ICONST_0
ISTORE 2
L1
LINENUMBER 297 L1
GETSTATIC scala/Predef$.MODULE$ : Lscala/Predef$;
ALOAD 0
INVOKEVIRTUAL org/apache/spark/mllib/linalg/DenseVector.values ()[D
INVOKEVIRTUAL scala/Predef$.doubleArrayOps ([D)Lscala/collection/mutable/ArrayOps;
INVOKEINTERFACE scala/collection/mutable/ArrayOps.size ()I
ISTORE 3
L2
LINENUMBER 298 L2
ALOAD 0
INVOKEVIRTUAL org/apache/spark/mllib/linalg/DenseVector.values ()[D
ASTORE 4
L3
LINENUMBER 299 L3
FRAME APPEND [I I [D]
ILOAD 2
ILOAD 3
IF_ICMPGE L4
L5
LINENUMBER 300 L5
ALOAD 1
NEW scala/Tuple2$mcID$sp
DUP
ILOAD 2
ALOAD 4
ILOAD 2
DALOAD
INVOKESPECIAL scala/Tuple2$mcID$sp.<init> (ID)V
INVOKEINTERFACE scala/Function1.apply (Ljava/lang/Object;)Ljava/lang/Object;
POP
L6
LINENUMBER 301 L6
ILOAD 2
ICONST_1
IADD
ISTORE 2
GOTO L3
```
However,
```
INVOKESPECIAL scala/Tuple2$mcID$sp.<init> (ID)V
INVOKEINTERFACE scala/Function1.apply (Ljava/lang/Object;)Ljava/lang/Object;
```
is expensive, so that's why checking zero in the anonymous function will slow down the whole thing.
I agree with you, the iterator is slow by nature, and we are only interested in foreach implementation. I'll remove the iterator, and just have foreach method in vector.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4431][MLlib] Implement efficient active...
Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on a diff in the pull request:
https://github.com/apache/spark/pull/3288#discussion_r20544650
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/Vectors.scala ---
@@ -273,6 +289,47 @@ class DenseVector(val values: Array[Double]) extends Vector {
override def copy: DenseVector = {
new DenseVector(values.clone())
}
+
+ private[spark] override def activeIterator(skippingZeros: Boolean) = new Iterator[(Int, Double)] {
+ private var i = 0
+ private val valuesSize = values.size
+
+ // If zeros are asked to be explicitly skipped, the parent `size` method is called to count
+ // the number of nonzero elements using `hasNext` and `next` methods.
+ override lazy val size: Int = if (skippingZeros) super.size else valuesSize
+
+ override def hasNext = {
+ if (skippingZeros) {
+ var found = false
+ while (!found && i < valuesSize) if (values(i) != 0.0) found = true else i += 1
+ }
+ i < valuesSize
+ }
+
+ override def next = {
+ val result = (i, values(i))
+ i += 1
+ result
+ }
+
+ override def foreach[@specialized(Unit) U](f: ((Int, Double)) => U) {
--- End diff --
Okay, the generated bytecode of both approach are the same.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org