[SPARK-1535] ALS: Avoid the garbage-creating ctor of DoubleMatrix #442
Conversation
|
Merged build triggered. |
|
Merged build started. |
|
Merged build finished. |
|
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/14226/ |
|
This appears to be a PySpark error unrelated to my change. |
|
+1 BTW I submitted a fix for this in jblas, although it is not yet released. |
| * Wrap a double array in a DoubleMatrix without creating garbage. | ||
| */ | ||
| private def wrapDoubleArray(v: Array[Double]): DoubleMatrix = { | ||
| new DoubleMatrix(v.length, 1, v:_*) |
There was a problem hiding this comment.
(PS might also note in the comment that it's safe to go back to the old DoubleMatrix constructor at jblas 1.2.4 or later.)
|
@tmyklebu Thanks for fixing this! Could you create a JIRA? |
|
Merged build triggered. |
|
Merged build started. |
|
Merged build finished. All automated tests passed. |
|
All automated tests passed. |
|
[SPARK-1535] describes the issue and the form of the fix. |
|
@tmyklebu Could you add the JIRA number to the title of this PR? It makes life easier for people who merge the code. |
tmyklebu
changed the title
|
LGTM. Thanks for fixing it! @mateiz Could you help merge this into both master and branch-1.0? |
|
@srowen: Does Hotspot actually generate code for the allocation and the dead store with the bad ctor? I haven't picked through it yet. |
|
I doubt Hotspot can remove this allocation, but am not sure. It would have to do a couple things -- inline the call to 2 other constructors, realize there's no threading issue in the constructor, realize the dead store. I think it's worth avoiding manually here. |
|
Merged into master and branch 1.0, thanks! |
`new DoubleMatrix(double[])` creates a garbage `double[]` of the same length as its argument and immediately throws it away. This pull request avoids that constructor in the ALS code. Author: Tor Myklebust <[email protected]> Closes #442 from tmyklebu/foo2 and squashes the following commits: 2784fc5 [Tor Myklebust] Mention that this is probably fixed as of jblas 1.2.4; repunctuate. a09904f [Tor Myklebust] Helper function for wrapping Array[Double]'s with DoubleMatrix's. (cherry picked from commit 25fc318) Signed-off-by: Matei Zaharia <[email protected]>
Workers should use working directory as spark home if it's not specified If users don't set SPARK_HOME in their environment file when launching an application, the standalone cluster should default to the spark home of the worker.
`new DoubleMatrix(double[])` creates a garbage `double[]` of the same length as its argument and immediately throws it away. This pull request avoids that constructor in the ALS code. Author: Tor Myklebust <[email protected]> Closes apache#442 from tmyklebu/foo2 and squashes the following commits: 2784fc5 [Tor Myklebust] Mention that this is probably fixed as of jblas 1.2.4; repunctuate. a09904f [Tor Myklebust] Helper function for wrapping Array[Double]'s with DoubleMatrix's.
Workers should use working directory as spark home if it's not specified If users don't set SPARK_HOME in their environment file when launching an application, the standalone cluster should default to the spark home of the worker. (cherry picked from commit 59f475c) Signed-off-by: Patrick Wendell <[email protected]>
Required for ./dev/check-license to pass
Fix error of running trovestack in openlab job
new DoubleMatrix(double[])creates a garbagedouble[]of the same length as its argument and immediately throws it away. This pull request avoids that constructor in the ALS code.