[SPARK-9437][core] avoid overflow in SizeEstimator

[squito]

https://issues.apache.org/jira/browse/SPARK-9437

[squito]

any suggestions on a test that doesn't require 2 GB of memory?

[shivaram]

LGTM. Could we also check all the other instances where alignSize is called ?

[SparkQA]

Test build #38853 has finished for PR 7750 at commit bc1cb82.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[squito]

@shivaram good idea. I updated one more spot where it seems there could be overflow -- I will admit I don't completely understand what is going on there, but the change seems safe. the only other place its called is https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/util/SizeEstimator.scala#L217 which is not going to overflow.

[srowen]

Does the val size = math.min(s1, s2) line need toLong too? it gets multiplied by a big number before being divided by one. I could be missing why it's not needed or can't be done but seemed like a possible issue.

[squito]

s1 & s2 are already longs, which means so is size -- so no change needed there.
(not that I had checked this before you mentioned it ...)

[shivaram]

Yeah its tricky to add a test case for this -- We could do it by refactoring the code and testing a smaller function which takes in the array size as an argument etc. but that'll be a much bigger code change. Since this is a more contained change I'd actually recommend just merging the current diff for now.

[SparkQA]

Test build #38877 has finished for PR 7750 at commit 29493f1.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[squito]

Jenkins, retest this please

[SparkQA]

Test build #149 has finished for PR 7750 at commit 29493f1.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #38932 has finished for PR 7750 at commit 29493f1.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[squito]

Jenkins, retest this please

[SparkQA]

Test build #162 has finished for PR 7750 at commit 29493f1.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #39039 has finished for PR 7750 at commit 29493f1.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[shivaram]

LGTM. Merging this