Hadoop Common
  1. Hadoop Common
  2. HADOOP-8084

Protobuf RPC engine can be optimized to not do copying for the RPC request/response

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.24.0
    • Fix Version/s: 2.0.0-alpha
    • Component/s: ipc
    • Labels:
      None

      Issue Links

        Activity

        Hide
        Devaraj Das added a comment -

        Straightforward patch. This is relevant to mapreduce too.

        Show
        Devaraj Das added a comment - Straightforward patch. This is relevant to mapreduce too.
        Hide
        Aaron T. Myers added a comment -

        The patch makes sense to me. Have you done any benchmarks? Perhaps using the tool introduced in HADOOP-8070?

        Show
        Aaron T. Myers added a comment - The patch makes sense to me. Have you done any benchmarks? Perhaps using the tool introduced in HADOOP-8070 ?
        Hide
        Devaraj Das added a comment -

        No I haven't done benchmarks. But it seemed to make sense to avoid a copy if it could be avoided.

        Show
        Devaraj Das added a comment - No I haven't done benchmarks. But it seemed to make sense to avoid a copy if it could be avoided.
        Hide
        Todd Lipcon added a comment -

        [oops, put comment in the wrong field.. duh]

        patch looks good, marking patch available so Hudson runs

        Show
        Todd Lipcon added a comment - [oops, put comment in the wrong field.. duh] patch looks good, marking patch available so Hudson runs
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12514923/protobuf-no-copy.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in .

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/615//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/615//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12514923/protobuf-no-copy.patch against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in . +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/615//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/615//console This message is automatically generated.
        Hide
        Devaraj Das added a comment -

        Committed to trunk. Thanks Aaron and Todd for the reviews.

        Show
        Devaraj Das added a comment - Committed to trunk. Thanks Aaron and Todd for the reviews.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk-Commit #1833 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/1833/)
        HADOOP-8084. Updates ProtoBufRpc engine to not do an unnecessary copy for RPC request/response. Contributed by Devaraj Das. (Revision 1291602)

        Result = SUCCESS
        ddas : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1291602
        Files :

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/ProtobufRpcEngine.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #1833 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/1833/ ) HADOOP-8084 . Updates ProtoBufRpc engine to not do an unnecessary copy for RPC request/response. Contributed by Devaraj Das. (Revision 1291602) Result = SUCCESS ddas : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1291602 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/ProtobufRpcEngine.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Common-trunk-Commit #1759 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/1759/)
        HADOOP-8084. Updates ProtoBufRpc engine to not do an unnecessary copy for RPC request/response. Contributed by Devaraj Das. (Revision 1291602)

        Result = SUCCESS
        ddas : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1291602
        Files :

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/ProtobufRpcEngine.java
        Show
        Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #1759 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/1759/ ) HADOOP-8084 . Updates ProtoBufRpc engine to not do an unnecessary copy for RPC request/response. Contributed by Devaraj Das. (Revision 1291602) Result = SUCCESS ddas : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1291602 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/ProtobufRpcEngine.java
        Hide
        Mahadev konar added a comment -

        Ddas,
        Can you merge this to the PB branch?

        Show
        Mahadev konar added a comment - Ddas, Can you merge this to the PB branch?
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk-Commit #1770 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/1770/)
        HADOOP-8084. Updates ProtoBufRpc engine to not do an unnecessary copy for RPC request/response. Contributed by Devaraj Das. (Revision 1291602)

        Result = ABORTED
        ddas : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1291602
        Files :

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/ProtobufRpcEngine.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #1770 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/1770/ ) HADOOP-8084 . Updates ProtoBufRpc engine to not do an unnecessary copy for RPC request/response. Contributed by Devaraj Das. (Revision 1291602) Result = ABORTED ddas : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1291602 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/ProtobufRpcEngine.java
        Hide
        Devaraj Das added a comment -

        Is that branch (branch-0.23-PB) scheduled to be merged with 0.23? This issue is currently marked for 0.24. I am okay merging this fix with that branch but wanted to double-check...

        Show
        Devaraj Das added a comment - Is that branch (branch-0.23-PB) scheduled to be merged with 0.23? This issue is currently marked for 0.24. I am okay merging this fix with that branch but wanted to double-check...
        Hide
        Mahadev konar added a comment -

        Check with Nicholas, he's been maintaining the PB branch.

        Show
        Mahadev konar added a comment - Check with Nicholas, he's been maintaining the PB branch.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk #962 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/962/)
        HADOOP-8084. Updates ProtoBufRpc engine to not do an unnecessary copy for RPC request/response. Contributed by Devaraj Das. (Revision 1291602)

        Result = FAILURE
        ddas : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1291602
        Files :

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/ProtobufRpcEngine.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #962 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/962/ ) HADOOP-8084 . Updates ProtoBufRpc engine to not do an unnecessary copy for RPC request/response. Contributed by Devaraj Das. (Revision 1291602) Result = FAILURE ddas : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1291602 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/ProtobufRpcEngine.java
        Hide
        Tsz Wo Nicholas Sze added a comment -

        Hi Devaraj and Mahadev,

        I will merge this to 0.23-PB. Thanks.

        Show
        Tsz Wo Nicholas Sze added a comment - Hi Devaraj and Mahadev, I will merge this to 0.23-PB. Thanks.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk #998 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/998/)
        HADOOP-8084. Updates ProtoBufRpc engine to not do an unnecessary copy for RPC request/response. Contributed by Devaraj Das. (Revision 1291602)

        Result = SUCCESS
        ddas : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1291602
        Files :

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/ProtobufRpcEngine.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #998 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/998/ ) HADOOP-8084 . Updates ProtoBufRpc engine to not do an unnecessary copy for RPC request/response. Contributed by Devaraj Das. (Revision 1291602) Result = SUCCESS ddas : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1291602 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/ProtobufRpcEngine.java
        Hide
        Tsz Wo Nicholas Sze added a comment -

        I have merged this to 0.23.

        Show
        Tsz Wo Nicholas Sze added a comment - I have merged this to 0.23.

          People

          • Assignee:
            Devaraj Das
            Reporter:
            Devaraj Das
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development