Skip to content
Closed
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ final class BufferHolder {

private static final int ARRAY_MAX = ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH;

// buffer is guarantee to be word-aligned since UnsafeRow assumes each field is word-aligned.
private byte[] buffer;
private int cursor = Platform.BYTE_ARRAY_OFFSET;
private final UnsafeRow row;
Expand All @@ -52,7 +53,8 @@ final class BufferHolder {
"too many fields (number of fields: " + row.numFields() + ")");
}
this.fixedSize = bitsetWidthInBytes + 8 * row.numFields();
this.buffer = new byte[fixedSize + initialSize];
int roundedSize = ByteArrayMethods.roundNumberOfBytesToNearestWord(fixedSize + initialSize);
this.buffer = new byte[roundedSize];
this.row = row;
this.row.pointTo(buffer, buffer.length);
}
Expand All @@ -61,6 +63,8 @@ final class BufferHolder {
* Grows the buffer by at least neededSize and points the row to the buffer.
*/
void grow(int neededSize) {
assert neededSize >= 0 :
"Cannot grow BufferHolder by size " + neededSize + " because the size is negative";
if (neededSize > ARRAY_MAX - totalSize()) {
throw new UnsupportedOperationException(
"Cannot grow BufferHolder by size " + neededSize + " because the size after growing " +
Expand All @@ -70,7 +74,8 @@ void grow(int neededSize) {
if (buffer.length < length) {
// This will not happen frequently, because the buffer is re-used.
int newLength = length < ARRAY_MAX / 2 ? length * 2 : ARRAY_MAX;
final byte[] tmp = new byte[newLength];
int roundedSize = ByteArrayMethods.roundNumberOfBytesToNearestWord(newLength);
final byte[] tmp = new byte[roundedSize];
Platform.copyMemory(
buffer,
Platform.BYTE_ARRAY_OFFSET,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ class BufferHolderSparkSubmitSuite
val argsForSparkSubmit = Seq(
"--class", BufferHolderSparkSubmitSuite.getClass.getName.stripSuffix("$"),
"--name", "SPARK-22222",
"--master", "local-cluster[2,1,1024]",
"--driver-memory", "4g",
"--master", "local-cluster[1,1,7168]",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we still support this?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good question. Several tests still seem to use local-cluster. Is it better to use local while it may require more memory?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@HyukjinKwon HyukjinKwon Aug 1, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we support this for testing purpose since, IIRC, that's going to make separate processes for workers.

"--driver-memory", "7g",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hm, just wondering if it's going to be problematic that the test now spawns a job that needs more than 7G of RAM? maybe I misunderstand.

"--conf", "spark.ui.enabled=false",
"--conf", "spark.master.rest.enabled=false",
"--conf", "spark.driver.extraJavaOptions=-ea",
Expand All @@ -55,22 +55,30 @@ object BufferHolderSparkSubmitSuite {

val ARRAY_MAX = ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH

val holder = new BufferHolder(new UnsafeRow(1000))
val unsafeRow = new UnsafeRow(1000)
val holder = new BufferHolder(unsafeRow)

holder.reset()
holder.grow(roundToWord(ARRAY_MAX / 2))

holder.reset()
holder.grow(roundToWord(ARRAY_MAX / 2 + 8))
// while to reuse a buffer may happen, this test checks whether the buffer can be grown
holder.grow(ARRAY_MAX / 2)
assert(unsafeRow.getSizeInBytes % 8 == 0)

holder.reset()
holder.grow(roundToWord(Integer.MAX_VALUE / 2))
holder.grow(ARRAY_MAX / 2 + 7)
assert(unsafeRow.getSizeInBytes % 8 == 0)

holder.reset()
holder.grow(roundToWord(Integer.MAX_VALUE))
}
holder.grow(Integer.MAX_VALUE / 2)
assert(unsafeRow.getSizeInBytes % 8 == 0)

holder.grow(ARRAY_MAX - holder.totalSize())
assert(unsafeRow.getSizeInBytes % 8 == 0)

private def roundToWord(len: Int): Int = {
ByteArrayMethods.roundNumberOfBytesToNearestWord(len)
try {
holder.grow(ARRAY_MAX + 1 - holder.totalSize())
assert(false)
} catch {
case _: UnsupportedOperationException => assert(true)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fix the indents here. assert(true) is a no-op, so just omit it. assert(false) is less useful than fail(...message...), above. Let an unexpected Throwable just fly out of the method to fail it rather than swallow it. But do you really just want to use intercept here?

case _: Throwable => assert(false)
}
}
}