You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by normanmaurer <gi...@git.apache.org> on 2018/08/14 17:08:29 UTC
[GitHub] spark pull request #22105: Eliminate extra memory copy done when a ByteBuf i...
GitHub user normanmaurer opened a pull request:
https://github.com/apache/spark/pull/22105
Eliminate extra memory copy done when a ByteBuf is used that is backe…
…d by > 1 ByteBuffer.
Motivation:
Calling ByteBuf.nioBuffer(...) can be costly when the ByteBuf is backed by more then 1 ByteBuf. In this case it makes more sense to call ByteBuf.nioBuffers(...) and iterate over the returned ByteBuffer[].
Modifications:
Check how many ByteBuffer are used and depending on it do either call nioBuffer(...) or nioBuffers(...)
Result:
Less memory copies. This is related to https://github.com/netty/netty/pull/8176.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/normanmaurer/spark composite_byte_buf_mem_copy
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/22105.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 #22105
----
commit 9cf11800ab59865d9860bca62de656a201f534ca
Author: Norman Maurer <no...@...>
Date: 2018-08-07T19:25:17Z
Eliminate extra memory copy done when a ByteBuf is used that is backed by > 1 ByteBuffer.
Motivation:
Calling ByteBuf.nioBuffer(...) can be costly when the ByteBuf is backed by more then 1 ByteBuf. In this case it makes more sense to call ByteBuf.nioBuffers(...) and iterate over the returned ByteBuffer[].
Modifications:
Check how many ByteBuffer are used and depending on it do either call nioBuffer(...) or nioBuffers(...)
Result:
Less memory copies. This is related to https://github.com/netty/netty/pull/8176.
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22105: [SPARK-25115] [Core] Eliminate extra memory copy done wh...
Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on the issue:
https://github.com/apache/spark/pull/22105
LGTM. Merged into master. Thanks.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22105: [SPARK-25115] [Core] Eliminate extra memory copy ...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22105#discussion_r210353176
--- Diff: common/network-common/src/main/java/org/apache/spark/network/protocol/MessageWithHeader.java ---
@@ -140,8 +140,24 @@ private int copyByteBuf(ByteBuf buf, WritableByteChannel target) throws IOExcept
// SPARK-24578: cap the sub-region's size of returned nio buffer to improve the performance
// for the case that the passed-in buffer has too many components.
int length = Math.min(buf.readableBytes(), NIO_BUFFER_LIMIT);
--- End diff --
That was only briefly discussed in the PR that Ryan linked above... the original code actually used 512k.
I think Hadoop's limit is a little low, but maybe 256k is a bit high. IIRC socket buffers are 32k by default on Linux, so it seems unlikely you'd be able to write 256k in one call (ignoring what IOUtil does internally). But maybe in practice it works ok.
If anyone has the time to test this out that would be great.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22105: [SPARK-25115] [Core] Eliminate extra memory copy ...
Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on a diff in the pull request:
https://github.com/apache/spark/pull/22105#discussion_r210354443
--- Diff: common/network-common/src/main/java/org/apache/spark/network/protocol/MessageWithHeader.java ---
@@ -140,8 +140,24 @@ private int copyByteBuf(ByteBuf buf, WritableByteChannel target) throws IOExcept
// SPARK-24578: cap the sub-region's size of returned nio buffer to improve the performance
// for the case that the passed-in buffer has too many components.
int length = Math.min(buf.readableBytes(), NIO_BUFFER_LIMIT);
--- End diff --
Maybe @normanmaurer has some insight?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22105: Eliminate extra memory copy done when a ByteBuf is used ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22105
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22105: [SPARK-25115] [Core] Eliminate extra memory copy ...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22105#discussion_r210336932
--- Diff: common/network-common/src/main/java/org/apache/spark/network/protocol/MessageWithHeader.java ---
@@ -140,8 +140,24 @@ private int copyByteBuf(ByteBuf buf, WritableByteChannel target) throws IOExcept
// SPARK-24578: cap the sub-region's size of returned nio buffer to improve the performance
// for the case that the passed-in buffer has too many components.
int length = Math.min(buf.readableBytes(), NIO_BUFFER_LIMIT);
--- End diff --
I re-read that discussion and you're right. I know this has been checked in, but the comment is now stale; the limit is there because of the behavior of the JRE code, not because of the composite buffer. It would be good to update it.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22105: [SPARK-25115] [Core] Eliminate extra memory copy done wh...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22105
**[Test build #94756 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94756/testReport)** for PR 22105 at commit [`beb1454`](https://github.com/apache/spark/commit/beb1454bbfeaf0ee2b6fbc9504d599db6056ddc8).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22105: [SPARK-25115] [Core] Eliminate extra memory copy done wh...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22105
**[Test build #94755 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94755/testReport)** for PR 22105 at commit [`2ccca98`](https://github.com/apache/spark/commit/2ccca98dc685e08ff32de2973d57dedbe27d4371).
* This patch **fails from timeout after a configured wait of \`340m\`**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22105: [SPARK-25115] [Core] Eliminate extra memory copy done wh...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22105
**[Test build #94755 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94755/testReport)** for PR 22105 at commit [`2ccca98`](https://github.com/apache/spark/commit/2ccca98dc685e08ff32de2973d57dedbe27d4371).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22105: Eliminate extra memory copy done when a ByteBuf is used ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22105
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22105: [SPARK-25115] [Core] Eliminate extra memory copy ...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22105#discussion_r210134223
--- Diff: common/network-common/src/main/java/org/apache/spark/network/protocol/MessageWithHeader.java ---
@@ -140,8 +140,24 @@ private int copyByteBuf(ByteBuf buf, WritableByteChannel target) throws IOExcept
// SPARK-24578: cap the sub-region's size of returned nio buffer to improve the performance
// for the case that the passed-in buffer has too many components.
int length = Math.min(buf.readableBytes(), NIO_BUFFER_LIMIT);
--- End diff --
My understanding is that the code here is avoiding the copy by using `nioBuffers()` already, so that limit shouldn't be needed, right? Or maybe I'm missing something.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22105: [SPARK-25115] [Core] Eliminate extra memory copy ...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/22105#discussion_r210056504
--- Diff: common/network-common/src/main/java/org/apache/spark/network/protocol/MessageWithHeader.java ---
@@ -140,8 +140,24 @@ private int copyByteBuf(ByteBuf buf, WritableByteChannel target) throws IOExcept
// SPARK-24578: cap the sub-region's size of returned nio buffer to improve the performance
// for the case that the passed-in buffer has too many components.
int length = Math.min(buf.readableBytes(), NIO_BUFFER_LIMIT);
- ByteBuffer buffer = buf.nioBuffer(buf.readerIndex(), length);
- int written = target.write(buffer);
+ // If the ByteBuf holds more then one ByteBuffer we should better call nioBuffers(...)
+ // to eliminate extra memory copies.
+ int written = 0;
+ if (buf.nioBufferCount() == 1) {
+ ByteBuffer buffer = buf.nioBuffer(buf.readerIndex(), length);
+ written = target.write(buffer);
+ } else {
+ ByteBuffer[] buffers = buf.nioBuffers(buf.readerIndex(), length);
+ for (ByteBuffer buffer: buffers) {
+ int remaining = buffer.remaining();
+ int w = target.write(buffer);
+ written += w;
--- End diff --
Can we guarantee `written` does not overflow while we accumulate `int` values?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22105: [SPARK-25115] [Core] Eliminate extra memory copy ...
Posted by xuanyuanking <gi...@git.apache.org>.
Github user xuanyuanking commented on a diff in the pull request:
https://github.com/apache/spark/pull/22105#discussion_r210842394
--- Diff: common/network-common/src/main/java/org/apache/spark/network/protocol/MessageWithHeader.java ---
@@ -140,8 +140,24 @@ private int copyByteBuf(ByteBuf buf, WritableByteChannel target) throws IOExcept
// SPARK-24578: cap the sub-region's size of returned nio buffer to improve the performance
// for the case that the passed-in buffer has too many components.
int length = Math.min(buf.readableBytes(), NIO_BUFFER_LIMIT);
--- End diff --
```
IIRC socket buffers are 32k by default on Linux, so it seems unlikely you'd be able to write 256k in one call (ignoring what IOUtil does internally). But maybe in practice it works ok.
```
After reading the context in #12083 and this discussion, I want to provide a possibility about 256k in one call can work in practice. As in our scenario, user will change `/proc/sys/net/core/wmem_default` based on their online behavior, generally we'll set this value larger than `wmem_default`.
So maybe 256k of NIO_BUFFER_LIMIT is ok here? We just need add more annotation to remind what params related with this value.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22105: [SPARK-25115] [Core] Eliminate extra memory copy done wh...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22105
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22105: [SPARK-25115] [Core] Eliminate extra memory copy ...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22105#discussion_r210082445
--- Diff: common/network-common/src/main/java/org/apache/spark/network/protocol/MessageWithHeader.java ---
@@ -140,8 +140,24 @@ private int copyByteBuf(ByteBuf buf, WritableByteChannel target) throws IOExcept
// SPARK-24578: cap the sub-region's size of returned nio buffer to improve the performance
// for the case that the passed-in buffer has too many components.
int length = Math.min(buf.readableBytes(), NIO_BUFFER_LIMIT);
--- End diff --
Is this limit needed now at all? I think your change was suggested as a future enhancement when reviewing SPARK-24578 but I never noticed a PR to implement it until yours...
@attilapiros
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22105: [SPARK-25115] [Core] Eliminate extra memory copy done wh...
Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on the issue:
https://github.com/apache/spark/pull/22105
+ @squito who reviewed this code before.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22105: Eliminate extra memory copy done when a ByteBuf is used ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22105
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/94753/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22105: [SPARK-25115] [Core] Eliminate extra memory copy ...
Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on a diff in the pull request:
https://github.com/apache/spark/pull/22105#discussion_r210350203
--- Diff: common/network-common/src/main/java/org/apache/spark/network/protocol/MessageWithHeader.java ---
@@ -140,8 +140,24 @@ private int copyByteBuf(ByteBuf buf, WritableByteChannel target) throws IOExcept
// SPARK-24578: cap the sub-region's size of returned nio buffer to improve the performance
// for the case that the passed-in buffer has too many components.
int length = Math.min(buf.readableBytes(), NIO_BUFFER_LIMIT);
--- End diff --
Out of my curiosity, how do we come out with number of NIO_BUFFER_LIMIT 256KB?
In Hadoop, they are using [8KB](https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java#L3234)
For most OSes, in the `write(ByteBuffer[])` API in `sun.nio.ch.IOUtil`, it goes one buffer at a time, and gets a temporary direct buffer from the `BufferCache`, up to a limit of `IOUtil#IOV_MAX` which is 1KB.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22105: Eliminate extra memory copy done when a ByteBuf is used ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22105
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22105: Eliminate extra memory copy done when a ByteBuf is used ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22105
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22105: [SPARK-25115] [Core] Eliminate extra memory copy done wh...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22105
**[Test build #94756 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94756/testReport)** for PR 22105 at commit [`beb1454`](https://github.com/apache/spark/commit/beb1454bbfeaf0ee2b6fbc9504d599db6056ddc8).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22105: [SPARK-25115] [Core] Eliminate extra memory copy done wh...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22105
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/94756/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22105: [SPARK-25115] [Core] Eliminate extra memory copy done wh...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22105
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/94755/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22105: [SPARK-25115] [Core] Eliminate extra memory copy ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/22105
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22105: [SPARK-25115] [Core] Eliminate extra memory copy done wh...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22105
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22105: [SPARK-25115] [Core] Eliminate extra memory copy ...
Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on a diff in the pull request:
https://github.com/apache/spark/pull/22105#discussion_r210080636
--- Diff: common/network-common/src/main/java/org/apache/spark/network/protocol/MessageWithHeader.java ---
@@ -140,8 +140,24 @@ private int copyByteBuf(ByteBuf buf, WritableByteChannel target) throws IOExcept
// SPARK-24578: cap the sub-region's size of returned nio buffer to improve the performance
// for the case that the passed-in buffer has too many components.
int length = Math.min(buf.readableBytes(), NIO_BUFFER_LIMIT);
- ByteBuffer buffer = buf.nioBuffer(buf.readerIndex(), length);
- int written = target.write(buffer);
+ // If the ByteBuf holds more then one ByteBuffer we should better call nioBuffers(...)
+ // to eliminate extra memory copies.
+ int written = 0;
+ if (buf.nioBufferCount() == 1) {
+ ByteBuffer buffer = buf.nioBuffer(buf.readerIndex(), length);
+ written = target.write(buffer);
+ } else {
+ ByteBuffer[] buffers = buf.nioBuffers(buf.readerIndex(), length);
+ for (ByteBuffer buffer: buffers) {
+ int remaining = buffer.remaining();
+ int w = target.write(buffer);
+ written += w;
--- End diff --
We are using `Int` in the current codebase, so this should not be a concern. Also, `ByteBuf` is using `Int` to index the byte; as a result, it's impossible to overflow `written` given they're from the same `ByteBuf`.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22105: [SPARK-25115] [Core] Eliminate extra memory copy done wh...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the issue:
https://github.com/apache/spark/pull/22105
@normanmaurer LGTM. Thanks for the fix. I totally forgot this issue.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22105: [SPARK-25115] [Core] Eliminate extra memory copy ...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/22105#discussion_r210133826
--- Diff: common/network-common/src/main/java/org/apache/spark/network/protocol/MessageWithHeader.java ---
@@ -140,8 +140,24 @@ private int copyByteBuf(ByteBuf buf, WritableByteChannel target) throws IOExcept
// SPARK-24578: cap the sub-region's size of returned nio buffer to improve the performance
// for the case that the passed-in buffer has too many components.
int length = Math.min(buf.readableBytes(), NIO_BUFFER_LIMIT);
--- End diff --
@vanzin This is to avoid memory copy when writing a large ByteBuffer. You merged this actually: #12083
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22105: [SPARK-25115] [Core] Eliminate extra memory copy ...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/22105#discussion_r210134581
--- Diff: common/network-common/src/main/java/org/apache/spark/network/protocol/MessageWithHeader.java ---
@@ -140,8 +140,24 @@ private int copyByteBuf(ByteBuf buf, WritableByteChannel target) throws IOExcept
// SPARK-24578: cap the sub-region's size of returned nio buffer to improve the performance
// for the case that the passed-in buffer has too many components.
int length = Math.min(buf.readableBytes(), NIO_BUFFER_LIMIT);
--- End diff --
@vanzin The ByteBuffer here may be just a large ByteBuffer. See my comment here: https://github.com/apache/spark/pull/12083#issuecomment-204499691
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22105: Eliminate extra memory copy done when a ByteBuf is used ...
Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on the issue:
https://github.com/apache/spark/pull/22105
add to whitelist
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22105: [SPARK-25115] [Core] Eliminate extra memory copy ...
Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on a diff in the pull request:
https://github.com/apache/spark/pull/22105#discussion_r210090252
--- Diff: common/network-common/src/main/java/org/apache/spark/network/protocol/MessageWithHeader.java ---
@@ -140,8 +140,24 @@ private int copyByteBuf(ByteBuf buf, WritableByteChannel target) throws IOExcept
// SPARK-24578: cap the sub-region's size of returned nio buffer to improve the performance
// for the case that the passed-in buffer has too many components.
int length = Math.min(buf.readableBytes(), NIO_BUFFER_LIMIT);
--- End diff --
Agree, with this PR, seems this limit is not needed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org