Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      Block report processing can be more efficient if instances of the block ID class are replaced with a sequence of longs.

      1. brAsLongsPatch5.txt
        12 kB
        Sanjay Radia
      2. brAsLongsPatch3.txt
        11 kB
        Sanjay Radia
      3. comaprison.txt
        0.5 kB
        Sanjay Radia
      4. brAsLongsPatch2.txt
        11 kB
        Sanjay Radia

        Activity

        Hide
        Sanjay Radia added a comment -

        The attached patch uses an array of longs instead of an array of Class Block
        for the Block report (BR)
        A block is 2 longs (block id and block length).
        Java's overhead for an object is 16 bytes.
        For an array of objects each array entry is a separately allocated object.

        Comparison of the block report processing times.
        All times are in millisec and are measured from the data node sending the BR and
        receiving an acknowledgement from the NN that the BR was processed.

        Approx 50 improvement in the BR processing time

        BRs using Class Block || BRs using Longs
        Num Time Time || Time Time
        Blocks Total per 1K blocks || Total per 1K blocks
        --------------------------------------------||----------------------
        40000 651ms 16ms || 230ms 5.75ms
        100000 1258ms 12ms || 573ms 5.73ms
        300000 3521ms 12ms || 1800ms 6.00ms
        500000 5930ms 12ms || 2916ms 5.83ms

        Show
        Sanjay Radia added a comment - The attached patch uses an array of longs instead of an array of Class Block for the Block report (BR) A block is 2 longs (block id and block length). Java's overhead for an object is 16 bytes. For an array of objects each array entry is a separately allocated object. Comparison of the block report processing times. All times are in millisec and are measured from the data node sending the BR and receiving an acknowledgement from the NN that the BR was processed. Approx 50 improvement in the BR processing time BRs using Class Block || BRs using Longs Num Time Time || Time Time Blocks Total per 1K blocks || Total per 1K blocks -------------------------------------------- || ---------------------- 40000 651ms 16ms || 230ms 5.75ms 100000 1258ms 12ms || 573ms 5.73ms 300000 3521ms 12ms || 1800ms 6.00ms 500000 5930ms 12ms || 2916ms 5.83ms
        Hide
        Sanjay Radia added a comment -

        Sorry the formating of the comaprison table got messed up.
        Here the same in a attached file.

        Show
        Sanjay Radia added a comment - Sorry the formating of the comaprison table got messed up. Here the same in a attached file.
        Hide
        Doug Cutting added a comment -

        BlockListAsLongs should not be public.

        DatanodeProtocol's version should be incremented.

        Show
        Doug Cutting added a comment - BlockListAsLongs should not be public. DatanodeProtocol's version should be incremented.
        Hide
        Sanjay Radia added a comment -

        Updated patch

        • updated the protocol version for data node protocol
        • BlockListAsLongs is not public
        Show
        Sanjay Radia added a comment - Updated patch updated the protocol version for data node protocol BlockListAsLongs is not public
        Hide
        dhruba borthakur added a comment -

        1. In BlockListAsLongs() , you access iBlockList before checking whether it is null.
        2. Also, BlockListAsLongs() constructor checks th condition "if (iBlockList.length%LONGS_PER_BLOCK != 0)" twice.

        3. There should be no "public" methods in BlockListAsLongs. These methods should be package private.

        All other portions of code looks good. +1.

        Show
        dhruba borthakur added a comment - 1. In BlockListAsLongs() , you access iBlockList before checking whether it is null. 2. Also, BlockListAsLongs() constructor checks th condition "if (iBlockList.length%LONGS_PER_BLOCK != 0)" twice. 3. There should be no "public" methods in BlockListAsLongs. These methods should be package private. All other portions of code looks good. +1.
        Hide
        Sanjay Radia added a comment -

        Updated - fixed Dhruba's sugestins.

        Show
        Sanjay Radia added a comment - Updated - fixed Dhruba's sugestins.
        Hide
        dhruba borthakur added a comment -

        +1. Code looks good.

        Show
        dhruba borthakur added a comment - +1. Code looks good.
        Hide
        Sanjay Radia added a comment -

        Submitting my last patch: brAsLongsPatch5.txt

        Show
        Sanjay Radia added a comment - Submitting my last patch: brAsLongsPatch5.txt
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12372470/brAsLongsPatch5.txt
        against trunk revision .

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

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

        javac +1. The applied patch does not generate any new compiler warnings.

        findbugs +1. The patch does not introduce any new Findbugs warnings.

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

        contrib tests -1. The patch failed contrib unit tests.

        Test results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1463/testReport/
        Findbugs warnings: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1463/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1463/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1463/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/12372470/brAsLongsPatch5.txt against trunk revision . @author +1. The patch does not contain any @author tags. javadoc +1. The javadoc tool did not generate any warning messages. javac +1. The applied patch does not generate any new compiler warnings. findbugs +1. The patch does not introduce any new Findbugs warnings. core tests +1. The patch passed core unit tests. contrib tests -1. The patch failed contrib unit tests. Test results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1463/testReport/ Findbugs warnings: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1463/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1463/artifact/trunk/build/test/checkstyle-errors.html Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1463/console This message is automatically generated.
        Hide
        dhruba borthakur added a comment -

        I just committed this. Thanks Sanjay!

        Show
        dhruba borthakur added a comment - I just committed this. Thanks Sanjay!

          People

          • Assignee:
            Sanjay Radia
            Reporter:
            Robert Chansler
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development