Uploaded image for project: 'Hadoop HDFS'
  1. Hadoop HDFS
  2. HDFS-11383

Intern strings in BlockLocation and ExtendedBlock

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.9.0, 3.0.0-alpha4
    • Component/s: None
    • Labels:
      None

      Description

      I am working on Hive performance, investigating the problem of high memory pressure when (a) a table consists of a high number (thousands) of partitions and (b) multiple queries run against it concurrently. It turns out that a lot of memory is wasted due to data duplication. One source of duplicate strings is class org.apache.hadoop.fs.BlockLocation. Its fields such as storageIds, topologyPaths, hosts, names, may collectively use up to 6% of memory in my benchmark, causing (together with other problematic classes) a huge memory spike. Of these 6% of memory taken by BlockLocation strings, more than 5% are wasted due to duplication.

      I think we need to add calls to String.intern() in the BlockLocation constructor, like:

      this.hosts = internStringsInArray(hosts);
      ...
      
      private void internStringsInArray(String[] sar) {
        for (int i = 0; i < sar.length; i++) {
          sar[i] = sar[i].intern();
        }
      }
      

      String.intern() performs very well starting from JDK 7. I've found some articles explaining the progress that was made by the HotSpot JVM developers in this area, verified that with benchmarks myself, and finally added quite a bit of interning to one of the Cloudera products without any issues.

      1. HDFS-11383.01.patch
        6 kB
        Misha Dmitriev
      2. HDFS-11383.02.patch
        7 kB
        Misha Dmitriev
      3. HDFS-11383.03.patch
        7 kB
        Misha Dmitriev
      4. HDFS-11383.04.patch
        8 kB
        Misha Dmitriev
      5. hs2-crash-2.txt
        57 kB
        Misha Dmitriev

        Activity

        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11815 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11815/)
        HDFS-11383. Intern strings in BlockLocation and ExtendedBlock. (wang: rev 7101477e4726a70ab0eab57c2d4480a04446a8dc)

        • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/StringInterner.java
        • (edit) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocol/ExtendedBlock.java
        • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/BlockLocation.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11815 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11815/ ) HDFS-11383 . Intern strings in BlockLocation and ExtendedBlock. (wang: rev 7101477e4726a70ab0eab57c2d4480a04446a8dc) (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/StringInterner.java (edit) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocol/ExtendedBlock.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/BlockLocation.java
        Hide
        andrew.wang Andrew Wang added a comment -

        Committed to trunk and branch-2, thanks again Misha for working on this!

        Show
        andrew.wang Andrew Wang added a comment - Committed to trunk and branch-2, thanks again Misha for working on this!
        Hide
        andrew.wang Andrew Wang added a comment -

        LGTM thanks Misha, will commit shortly.

        Show
        andrew.wang Andrew Wang added a comment - LGTM thanks Misha, will commit shortly.
        Hide
        misha@cloudera.com Misha Dmitriev added a comment -

        Hi Andrew, I think this time I've addressed all your concerns and there is
        nothing from findbugs or checkstyle. Could you please merge this patch?

        On Wed, May 31, 2017 at 11:00 AM, Andrew Wang (JIRA) <jira@apache.org>

        Show
        misha@cloudera.com Misha Dmitriev added a comment - Hi Andrew, I think this time I've addressed all your concerns and there is nothing from findbugs or checkstyle. Could you please merge this patch? On Wed, May 31, 2017 at 11:00 AM, Andrew Wang (JIRA) <jira@apache.org>
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 29s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 test4tests 0m 0s 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.
        0 mvndep 0m 19s Maven dependency ordering for branch
        +1 mvninstall 15m 11s trunk passed
        +1 compile 14m 52s trunk passed
        +1 checkstyle 2m 2s trunk passed
        +1 mvnsite 1m 55s trunk passed
        +1 mvneclipse 0m 41s trunk passed
        -1 findbugs 1m 28s hadoop-common-project/hadoop-common in trunk has 19 extant Findbugs warnings.
        +1 javadoc 1m 20s trunk passed
        0 mvndep 0m 16s Maven dependency ordering for patch
        +1 mvninstall 1m 15s the patch passed
        +1 compile 14m 32s the patch passed
        +1 javac 14m 32s the patch passed
        +1 checkstyle 1m 58s root: The patch generated 0 new + 23 unchanged - 2 fixed = 23 total (was 25)
        +1 mvnsite 1m 54s the patch passed
        +1 mvneclipse 0m 38s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 3m 40s the patch passed
        +1 javadoc 1m 20s the patch passed
        +1 unit 8m 34s hadoop-common in the patch passed.
        +1 unit 1m 38s hadoop-hdfs-client in the patch passed.
        +1 asflicense 0m 36s The patch does not generate ASF License warnings.
        77m 26s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:14b5c93
        JIRA Issue HDFS-11383
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12870664/HDFS-11383.04.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 03e044db727f 3.13.0-108-generic #155-Ubuntu SMP Wed Jan 11 16:58:52 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / d5b71e4
        Default Java 1.8.0_131
        findbugs v3.1.0-RC1
        findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/19708/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-common-warnings.html
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/19708/testReport/
        modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client U: .
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/19708/console
        Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 29s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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. 0 mvndep 0m 19s Maven dependency ordering for branch +1 mvninstall 15m 11s trunk passed +1 compile 14m 52s trunk passed +1 checkstyle 2m 2s trunk passed +1 mvnsite 1m 55s trunk passed +1 mvneclipse 0m 41s trunk passed -1 findbugs 1m 28s hadoop-common-project/hadoop-common in trunk has 19 extant Findbugs warnings. +1 javadoc 1m 20s trunk passed 0 mvndep 0m 16s Maven dependency ordering for patch +1 mvninstall 1m 15s the patch passed +1 compile 14m 32s the patch passed +1 javac 14m 32s the patch passed +1 checkstyle 1m 58s root: The patch generated 0 new + 23 unchanged - 2 fixed = 23 total (was 25) +1 mvnsite 1m 54s the patch passed +1 mvneclipse 0m 38s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 40s the patch passed +1 javadoc 1m 20s the patch passed +1 unit 8m 34s hadoop-common in the patch passed. +1 unit 1m 38s hadoop-hdfs-client in the patch passed. +1 asflicense 0m 36s The patch does not generate ASF License warnings. 77m 26s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue HDFS-11383 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12870664/HDFS-11383.04.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 03e044db727f 3.13.0-108-generic #155-Ubuntu SMP Wed Jan 11 16:58:52 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / d5b71e4 Default Java 1.8.0_131 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/19708/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-common-warnings.html Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/19708/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client U: . Console output https://builds.apache.org/job/PreCommit-HDFS-Build/19708/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        misha@cloudera.com Misha Dmitriev added a comment -

        Ok, I've fixed checkstyles and added HashCodeBuilder. Will upload the new patch in a moment.

        Show
        misha@cloudera.com Misha Dmitriev added a comment - Ok, I've fixed checkstyles and added HashCodeBuilder. Will upload the new patch in a moment.
        Hide
        andrew.wang Andrew Wang added a comment -

        LGTM, though we need to fix the checkstyles. I won't force you to use the builders, but I think it would be more readable, particularly the long line that checkstyle flagged in the equals method. Please consider it.

        Show
        andrew.wang Andrew Wang added a comment - LGTM, though we need to fix the checkstyles. I won't force you to use the builders, but I think it would be more readable, particularly the long line that checkstyle flagged in the equals method. Please consider it.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 20s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 test4tests 0m 0s 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.
        0 mvndep 1m 25s Maven dependency ordering for branch
        +1 mvninstall 12m 32s trunk passed
        +1 compile 12m 45s trunk passed
        +1 checkstyle 1m 35s trunk passed
        +1 mvnsite 1m 31s trunk passed
        +1 mvneclipse 0m 30s trunk passed
        -1 findbugs 1m 20s hadoop-common-project/hadoop-common in trunk has 19 extant Findbugs warnings.
        +1 javadoc 1m 9s trunk passed
        0 mvndep 0m 14s Maven dependency ordering for patch
        +1 mvninstall 1m 11s the patch passed
        +1 compile 12m 42s the patch passed
        +1 javac 12m 42s the patch passed
        -0 checkstyle 1m 38s root: The patch generated 5 new + 24 unchanged - 2 fixed = 29 total (was 26)
        +1 mvnsite 1m 35s the patch passed
        +1 mvneclipse 0m 31s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 3m 1s the patch passed
        +1 javadoc 1m 8s the patch passed
        +1 unit 7m 20s hadoop-common in the patch passed.
        +1 unit 1m 17s hadoop-hdfs-client in the patch passed.
        +1 asflicense 0m 30s The patch does not generate ASF License warnings.
        66m 41s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:14b5c93
        JIRA Issue HDFS-11383
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12870474/HDFS-11383.03.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 92f4be1edf5e 4.4.0-43-generic #63-Ubuntu SMP Wed Oct 12 13:48:03 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 4b4a652
        Default Java 1.8.0_131
        findbugs v3.1.0-RC1
        findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/19680/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-common-warnings.html
        checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/19680/artifact/patchprocess/diff-checkstyle-root.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/19680/testReport/
        modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client U: .
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/19680/console
        Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 20s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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. 0 mvndep 1m 25s Maven dependency ordering for branch +1 mvninstall 12m 32s trunk passed +1 compile 12m 45s trunk passed +1 checkstyle 1m 35s trunk passed +1 mvnsite 1m 31s trunk passed +1 mvneclipse 0m 30s trunk passed -1 findbugs 1m 20s hadoop-common-project/hadoop-common in trunk has 19 extant Findbugs warnings. +1 javadoc 1m 9s trunk passed 0 mvndep 0m 14s Maven dependency ordering for patch +1 mvninstall 1m 11s the patch passed +1 compile 12m 42s the patch passed +1 javac 12m 42s the patch passed -0 checkstyle 1m 38s root: The patch generated 5 new + 24 unchanged - 2 fixed = 29 total (was 26) +1 mvnsite 1m 35s the patch passed +1 mvneclipse 0m 31s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 1s the patch passed +1 javadoc 1m 8s the patch passed +1 unit 7m 20s hadoop-common in the patch passed. +1 unit 1m 17s hadoop-hdfs-client in the patch passed. +1 asflicense 0m 30s The patch does not generate ASF License warnings. 66m 41s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue HDFS-11383 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12870474/HDFS-11383.03.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 92f4be1edf5e 4.4.0-43-generic #63-Ubuntu SMP Wed Oct 12 13:48:03 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 4b4a652 Default Java 1.8.0_131 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/19680/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-common-warnings.html checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/19680/artifact/patchprocess/diff-checkstyle-root.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/19680/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client U: . Console output https://builds.apache.org/job/PreCommit-HDFS-Build/19680/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        misha@cloudera.com Misha Dmitriev added a comment -

        I think since this code is not under active development and its data fields are unlikely to change, it will be easier for now to just make a small change to the existing code. HashCodeBuilder looks like a useful thing for a class with many diverse data fields. But one still needs to remember all the data fields to pass it as parameters (or use the alternative API that uses reflection internally, which should be damn slow for one thing...) So I think right here just fixing the existing hashCode() is a reasonable tradeoff, and I hope you will agree with me.

        Show
        misha@cloudera.com Misha Dmitriev added a comment - I think since this code is not under active development and its data fields are unlikely to change, it will be easier for now to just make a small change to the existing code. HashCodeBuilder looks like a useful thing for a class with many diverse data fields. But one still needs to remember all the data fields to pass it as parameters (or use the alternative API that uses reflection internally, which should be damn slow for one thing...) So I think right here just fixing the existing hashCode() is a reasonable tradeoff, and I hope you will agree with me.
        Hide
        andrew.wang Andrew Wang added a comment -

        Yea, maybe we should move to using an equals/hashCode builder? There's a nice one in Apache Commons, Guava too. I'm generally against manually implemented equals/hashCode since the builders exist.

        Show
        andrew.wang Andrew Wang added a comment - Yea, maybe we should move to using an equals/hashCode builder? There's a nice one in Apache Commons, Guava too. I'm generally against manually implemented equals/hashCode since the builders exist.
        Hide
        misha@cloudera.com Misha Dmitriev added a comment -

        Thank you Andrew. This findbugs warning makes sense, in principle, since there is an ExtendedBlock constructor that indeed sets poolId to null. I've just replaced all the simple 'poolId.intern()' statements with 'poolId != null ? poolId.intern() : null'. But now I see that hashCode() and equals() would fail if poolId is null. Do you think it makes sense to fix them just in case as well?

        Show
        misha@cloudera.com Misha Dmitriev added a comment - Thank you Andrew. This findbugs warning makes sense, in principle, since there is an ExtendedBlock constructor that indeed sets poolId to null. I've just replaced all the simple 'poolId.intern()' statements with 'poolId != null ? poolId.intern() : null'. But now I see that hashCode() and equals() would fail if poolId is null. Do you think it makes sense to fix them just in case as well?
        Hide
        andrew.wang Andrew Wang added a comment -

        Thanks for revving Misha, LGTM except we need to handle the new findbugs warning in ExtendedBlock. I think adding a null check will fix it.

        Show
        andrew.wang Andrew Wang added a comment - Thanks for revving Misha, LGTM except we need to handle the new findbugs warning in ExtendedBlock. I think adding a null check will fix it.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 15s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 test4tests 0m 0s 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.
        0 mvndep 1m 48s Maven dependency ordering for branch
        +1 mvninstall 13m 59s trunk passed
        +1 compile 15m 25s trunk passed
        +1 checkstyle 2m 0s trunk passed
        +1 mvnsite 2m 3s trunk passed
        +1 mvneclipse 0m 39s trunk passed
        -1 findbugs 1m 44s hadoop-common-project/hadoop-common in trunk has 19 extant Findbugs warnings.
        +1 javadoc 1m 14s trunk passed
        0 mvndep 0m 15s Maven dependency ordering for patch
        +1 mvninstall 1m 11s the patch passed
        +1 compile 12m 58s the patch passed
        +1 javac 12m 58s the patch passed
        -0 checkstyle 1m 56s root: The patch generated 4 new + 23 unchanged - 2 fixed = 27 total (was 25)
        +1 mvnsite 1m 47s the patch passed
        +1 mvneclipse 0m 39s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        -1 findbugs 1m 40s hadoop-hdfs-project/hadoop-hdfs-client generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
        +1 javadoc 1m 20s the patch passed
        -1 unit 8m 16s hadoop-common in the patch failed.
        +1 unit 1m 26s hadoop-hdfs-client in the patch passed.
        +1 asflicense 0m 32s The patch does not generate ASF License warnings.
        75m 41s



        Reason Tests
        FindBugs module:hadoop-hdfs-project/hadoop-hdfs-client
          Non-virtual method call in new org.apache.hadoop.hdfs.protocol.ExtendedBlock() passes null for non-null parameter of new ExtendedBlock(String, long, long, long) At ExtendedBlock.java:org.apache.hadoop.hdfs.protocol.ExtendedBlock() passes null for non-null parameter of new ExtendedBlock(String, long, long, long) At ExtendedBlock.java:[line 33]
        Failed junit tests hadoop.ha.TestZKFailoverController
          hadoop.security.TestRaceWhenRelogin
          hadoop.security.TestGroupsCaching



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:14b5c93
        JIRA Issue HDFS-11383
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12869963/HDFS-11383.02.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 4dfb36247ab5 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 2b5ad48
        Default Java 1.8.0_131
        findbugs v3.1.0-RC1
        findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/19621/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-common-warnings.html
        checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/19621/artifact/patchprocess/diff-checkstyle-root.txt
        findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/19621/artifact/patchprocess/new-findbugs-hadoop-hdfs-project_hadoop-hdfs-client.html
        unit https://builds.apache.org/job/PreCommit-HDFS-Build/19621/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/19621/testReport/
        modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client U: .
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/19621/console
        Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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. 0 mvndep 1m 48s Maven dependency ordering for branch +1 mvninstall 13m 59s trunk passed +1 compile 15m 25s trunk passed +1 checkstyle 2m 0s trunk passed +1 mvnsite 2m 3s trunk passed +1 mvneclipse 0m 39s trunk passed -1 findbugs 1m 44s hadoop-common-project/hadoop-common in trunk has 19 extant Findbugs warnings. +1 javadoc 1m 14s trunk passed 0 mvndep 0m 15s Maven dependency ordering for patch +1 mvninstall 1m 11s the patch passed +1 compile 12m 58s the patch passed +1 javac 12m 58s the patch passed -0 checkstyle 1m 56s root: The patch generated 4 new + 23 unchanged - 2 fixed = 27 total (was 25) +1 mvnsite 1m 47s the patch passed +1 mvneclipse 0m 39s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. -1 findbugs 1m 40s hadoop-hdfs-project/hadoop-hdfs-client generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) +1 javadoc 1m 20s the patch passed -1 unit 8m 16s hadoop-common in the patch failed. +1 unit 1m 26s hadoop-hdfs-client in the patch passed. +1 asflicense 0m 32s The patch does not generate ASF License warnings. 75m 41s Reason Tests FindBugs module:hadoop-hdfs-project/hadoop-hdfs-client   Non-virtual method call in new org.apache.hadoop.hdfs.protocol.ExtendedBlock() passes null for non-null parameter of new ExtendedBlock(String, long, long, long) At ExtendedBlock.java:org.apache.hadoop.hdfs.protocol.ExtendedBlock() passes null for non-null parameter of new ExtendedBlock(String, long, long, long) At ExtendedBlock.java: [line 33] Failed junit tests hadoop.ha.TestZKFailoverController   hadoop.security.TestRaceWhenRelogin   hadoop.security.TestGroupsCaching Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue HDFS-11383 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12869963/HDFS-11383.02.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 4dfb36247ab5 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 2b5ad48 Default Java 1.8.0_131 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/19621/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-common-warnings.html checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/19621/artifact/patchprocess/diff-checkstyle-root.txt findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/19621/artifact/patchprocess/new-findbugs-hadoop-hdfs-project_hadoop-hdfs-client.html unit https://builds.apache.org/job/PreCommit-HDFS-Build/19621/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/19621/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client U: . Console output https://builds.apache.org/job/PreCommit-HDFS-Build/19621/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        misha@cloudera.com Misha Dmitriev added a comment -

        Here is an excerpt from another jxray report, obtained from a real-life production run of Impala Catalog Server. It turns out that it wastes 36% of its memory due to duplicate strings, and of them at least 20% come from hadoop.fs.BlockLocation data fields. Some more duplicates come from other HDFS classes: org.apache.hadoop.hdfs.protocol.DatanodeInfoWithStorage and org.apache.hadoop.hdfs.protocol.ExtendedBlock.

        The second patch that I've just added has string interning added for ExtendedBlock.poolId data field.

        6. DUPLICATE STRINGS
        
        Total strings: 29,734,223  Unique strings: 3,011,444  Duplicate values: 309,566  Overhead: 2,388,231K (36.6%)
        
        ...
        
        ===================================================
        
        7. REFERENCE CHAINS FOR DUPLICATE STRINGS
        
          391,384K (6.0%), 3340329 dup strings (517 unique), 3340329 dup backing arrays:
             <-- String[] <-- org.apache.hadoop.fs.HdfsBlockLocation.storageIds <--  {j.u.ArrayList} <-- org.apache.impala.catalog.HdfsTable$FileBlocksInfo.locations <--  {j.u.HashMap}.values <-- Java Local (j.u.HashMap) [@631ce3948,@6890e01c8,@6a55b0778,@6ac1310f0] ... and 2 more GC roots (6 thread(s))
          365,345K (5.6%), 3340329 dup strings (28 unique), 3340329 dup backing arrays:
             <-- org.apache.hadoop.hdfs.protocol.DatanodeInfoWithStorage.datanodeUuid <-- org.apache.hadoop.hdfs.protocol.DatanodeInfoWithStorage[] <-- org.apache.hadoop.hdfs.protocol.LocatedBlock.locs <-- org.apache.hadoop.fs.HdfsBlockLocation.block <--  {j.u.ArrayList} <-- org.apache.impala.catalog.HdfsTable$FileBlocksInfo.locations <--  {j.u.HashMap}.values <-- Java Local (j.u.HashMap) [@631ce3948,@6890e01c8,@6a55b0778,@6ac1310f0] ... and 2 more GC roots (6 thread(s))
          328,625K (5.0%), 3340328 dup strings (28 unique), 3340328 dup backing arrays:
             <-- String[] <-- org.apache.hadoop.fs.HdfsBlockLocation.hosts <--  {j.u.ArrayList} <-- org.apache.impala.catalog.HdfsTable$FileBlocksInfo.locations <--  {j.u.HashMap}.values <-- Java Local (j.u.HashMap) [@631ce3948,@6890e01c8,@6a55b0778,@6ac1310f0] ... and 2 more GC roots (6 thread(s))
          313,153K (4.8%), 3340329 dup strings (28 unique), 3340329 dup backing arrays:
             <-- String[] <-- org.apache.hadoop.fs.HdfsBlockLocation.topologyPaths <--  {j.u.ArrayList} <-- org.apache.impala.catalog.HdfsTable$FileBlocksInfo.locations <--  {j.u.HashMap}.values <-- Java Local (j.u.HashMap) [@631ce3948,@6890e01c8,@6a55b0778,@6ac1310f0] ... and 2 more GC roots (6 thread(s))
          260,961K (4.0%), 3340329 dup strings (28 unique), 3340329 dup backing arrays:
             <-- String[] <-- org.apache.hadoop.fs.HdfsBlockLocation.names <--  {j.u.ArrayList} <-- org.apache.impala.catalog.HdfsTable$FileBlocksInfo.locations <--  {j.u.HashMap}.values <-- Java Local (j.u.HashMap) [@631ce3948,@6890e01c8,@6a55b0778,@6ac1310f0] ... and 2 more GC roots (6 thread(s))
          208,769K (3.2%), 3340329 dup strings (28 unique), 3340329 dup backing arrays:
             <-- org.apache.hadoop.hdfs.protocol.DatanodeInfoWithStorage.ipAddr <-- org.apache.hadoop.hdfs.protocol.DatanodeInfoWithStorage[] <-- org.apache.hadoop.hdfs.protocol.LocatedBlock.locs <-- org.apache.hadoop.fs.HdfsBlockLocation.block <--  {j.u.ArrayList} <-- org.apache.impala.catalog.HdfsTable$FileBlocksInfo.locations <--  {j.u.HashMap}.values <-- Java Local (j.u.HashMap) [@631ce3948,@6890e01c8,@6a55b0778,@6ac1310f0] ... and 2 more GC roots (6 thread(s))
          182,674K (2.8%), 3340329 dup strings (2 unique), 3340329 dup backing arrays:
             <-- org.apache.hadoop.hdfs.protocol.DatanodeInfoWithStorage.location <-- org.apache.hadoop.hdfs.protocol.DatanodeInfoWithStorage[] <-- org.apache.hadoop.hdfs.protocol.LocatedBlock.locs <-- org.apache.hadoop.fs.HdfsBlockLocation.block <--  {j.u.ArrayList} <-- org.apache.impala.catalog.HdfsTable$FileBlocksInfo.locations <--  {j.u.HashMap}.values <-- Java Local (j.u.HashMap) [@631ce3948,@6890e01c8,@6a55b0778,@6ac1310f0] ... and 2 more GC roots (6 thread(s))
          130,481K (2.0%), 1113443 dup strings (1 unique), 1113443 dup backing arrays:
             <-- org.apache.hadoop.hdfs.protocol.ExtendedBlock.poolId <-- org.apache.hadoop.hdfs.protocol.LocatedBlock.b <-- org.apache.hadoop.fs.HdfsBlockLocation.block <--  {j.u.ArrayList} <-- org.apache.impala.catalog.HdfsTable$FileBlocksInfo.locations <--  {j.u.HashMap}.values <-- Java Local (j.u.HashMap) [@631ce3948,@6890e01c8,@6a55b0778,@6ac1310f0] ... and 2 more GC roots (6 thread(s))
          59,974K (0.9%), 640705 dup strings (1000 unique), 640705 dup backing arrays:
             <--  {j.u.HashMap}.keys <--  {j.u.HashMap}.values <-- org.apache.impala.catalog.HdfsTable.perPartitionFileDescMap_ <-- Java Local (org.apache.impala.catalog.HdfsTable) [@631ce38b0,@6890e0130,@6a55b06e0,@6ac131058] ... and 2 more GC roots (6 thread(s))
          24,437K (0.4%), 252362 dup strings (14230 unique), 252362 dup backing arrays:
             <--  {j.u.HashMap}.keys <--  {j.u.HashMap}.values <-- org.apache.impala.catalog.HdfsTable.perPartitionFileDescMap_ <--  {java.util.concurrent.ConcurrentHashMap}.values <-- org.apache.impala.catalog.CatalogObjectCache.metadataCache_ <-- org.apache.impala.catalog.Db.tableCache_ <-- Java Local (org.apache.impala.catalog.Db) [@6015b2e90,@601c41930] (2 thread(s))
        
        Show
        misha@cloudera.com Misha Dmitriev added a comment - Here is an excerpt from another jxray report, obtained from a real-life production run of Impala Catalog Server. It turns out that it wastes 36% of its memory due to duplicate strings, and of them at least 20% come from hadoop.fs.BlockLocation data fields. Some more duplicates come from other HDFS classes: org.apache.hadoop.hdfs.protocol.DatanodeInfoWithStorage and org.apache.hadoop.hdfs.protocol.ExtendedBlock. The second patch that I've just added has string interning added for ExtendedBlock.poolId data field. 6. DUPLICATE STRINGS Total strings: 29,734,223 Unique strings: 3,011,444 Duplicate values: 309,566 Overhead: 2,388,231K (36.6%) ... =================================================== 7. REFERENCE CHAINS FOR DUPLICATE STRINGS 391,384K (6.0%), 3340329 dup strings (517 unique), 3340329 dup backing arrays: <-- String [] <-- org.apache.hadoop.fs.HdfsBlockLocation.storageIds <-- {j.u.ArrayList} <-- org.apache.impala.catalog.HdfsTable$FileBlocksInfo.locations <-- {j.u.HashMap}.values <-- Java Local (j.u.HashMap) [@631ce3948,@6890e01c8,@6a55b0778,@6ac1310f0] ... and 2 more GC roots (6 thread(s)) 365,345K (5.6%), 3340329 dup strings (28 unique), 3340329 dup backing arrays: <-- org.apache.hadoop.hdfs.protocol.DatanodeInfoWithStorage.datanodeUuid <-- org.apache.hadoop.hdfs.protocol.DatanodeInfoWithStorage[] <-- org.apache.hadoop.hdfs.protocol.LocatedBlock.locs <-- org.apache.hadoop.fs.HdfsBlockLocation.block <-- {j.u.ArrayList} <-- org.apache.impala.catalog.HdfsTable$FileBlocksInfo.locations <-- {j.u.HashMap}.values <-- Java Local (j.u.HashMap) [@631ce3948,@6890e01c8,@6a55b0778,@6ac1310f0] ... and 2 more GC roots (6 thread(s)) 328,625K (5.0%), 3340328 dup strings (28 unique), 3340328 dup backing arrays: <-- String [] <-- org.apache.hadoop.fs.HdfsBlockLocation.hosts <-- {j.u.ArrayList} <-- org.apache.impala.catalog.HdfsTable$FileBlocksInfo.locations <-- {j.u.HashMap}.values <-- Java Local (j.u.HashMap) [@631ce3948,@6890e01c8,@6a55b0778,@6ac1310f0] ... and 2 more GC roots (6 thread(s)) 313,153K (4.8%), 3340329 dup strings (28 unique), 3340329 dup backing arrays: <-- String [] <-- org.apache.hadoop.fs.HdfsBlockLocation.topologyPaths <-- {j.u.ArrayList} <-- org.apache.impala.catalog.HdfsTable$FileBlocksInfo.locations <-- {j.u.HashMap}.values <-- Java Local (j.u.HashMap) [@631ce3948,@6890e01c8,@6a55b0778,@6ac1310f0] ... and 2 more GC roots (6 thread(s)) 260,961K (4.0%), 3340329 dup strings (28 unique), 3340329 dup backing arrays: <-- String [] <-- org.apache.hadoop.fs.HdfsBlockLocation.names <-- {j.u.ArrayList} <-- org.apache.impala.catalog.HdfsTable$FileBlocksInfo.locations <-- {j.u.HashMap}.values <-- Java Local (j.u.HashMap) [@631ce3948,@6890e01c8,@6a55b0778,@6ac1310f0] ... and 2 more GC roots (6 thread(s)) 208,769K (3.2%), 3340329 dup strings (28 unique), 3340329 dup backing arrays: <-- org.apache.hadoop.hdfs.protocol.DatanodeInfoWithStorage.ipAddr <-- org.apache.hadoop.hdfs.protocol.DatanodeInfoWithStorage[] <-- org.apache.hadoop.hdfs.protocol.LocatedBlock.locs <-- org.apache.hadoop.fs.HdfsBlockLocation.block <-- {j.u.ArrayList} <-- org.apache.impala.catalog.HdfsTable$FileBlocksInfo.locations <-- {j.u.HashMap}.values <-- Java Local (j.u.HashMap) [@631ce3948,@6890e01c8,@6a55b0778,@6ac1310f0] ... and 2 more GC roots (6 thread(s)) 182,674K (2.8%), 3340329 dup strings (2 unique), 3340329 dup backing arrays: <-- org.apache.hadoop.hdfs.protocol.DatanodeInfoWithStorage.location <-- org.apache.hadoop.hdfs.protocol.DatanodeInfoWithStorage[] <-- org.apache.hadoop.hdfs.protocol.LocatedBlock.locs <-- org.apache.hadoop.fs.HdfsBlockLocation.block <-- {j.u.ArrayList} <-- org.apache.impala.catalog.HdfsTable$FileBlocksInfo.locations <-- {j.u.HashMap}.values <-- Java Local (j.u.HashMap) [@631ce3948,@6890e01c8,@6a55b0778,@6ac1310f0] ... and 2 more GC roots (6 thread(s)) 130,481K (2.0%), 1113443 dup strings (1 unique), 1113443 dup backing arrays: <-- org.apache.hadoop.hdfs.protocol.ExtendedBlock.poolId <-- org.apache.hadoop.hdfs.protocol.LocatedBlock.b <-- org.apache.hadoop.fs.HdfsBlockLocation.block <-- {j.u.ArrayList} <-- org.apache.impala.catalog.HdfsTable$FileBlocksInfo.locations <-- {j.u.HashMap}.values <-- Java Local (j.u.HashMap) [@631ce3948,@6890e01c8,@6a55b0778,@6ac1310f0] ... and 2 more GC roots (6 thread(s)) 59,974K (0.9%), 640705 dup strings (1000 unique), 640705 dup backing arrays: <-- {j.u.HashMap}.keys <-- {j.u.HashMap}.values <-- org.apache.impala.catalog.HdfsTable.perPartitionFileDescMap_ <-- Java Local (org.apache.impala.catalog.HdfsTable) [@631ce38b0,@6890e0130,@6a55b06e0,@6ac131058] ... and 2 more GC roots (6 thread(s)) 24,437K (0.4%), 252362 dup strings (14230 unique), 252362 dup backing arrays: <-- {j.u.HashMap}.keys <-- {j.u.HashMap}.values <-- org.apache.impala.catalog.HdfsTable.perPartitionFileDescMap_ <-- {java.util.concurrent.ConcurrentHashMap}.values <-- org.apache.impala.catalog.CatalogObjectCache.metadataCache_ <-- org.apache.impala.catalog.Db.tableCache_ <-- Java Local (org.apache.impala.catalog.Db) [@6015b2e90,@601c41930] (2 thread(s))
        Hide
        misha@cloudera.com Misha Dmitriev added a comment -

        Hi Andrew,

        I understand your concerns. Unit tests could be a good solution, but the problem is, to quantify the effect of a change like that one would need, in principle, to first run some code that uses BlockLocation unchanged and measure how much memory is consumed, then run the same code with BlockLocation that has interning and measure memory again. There is also a problem of how representative such a "pseudo-benchmark" would be, e.g. I can easily populate some data structure with very big strings and then demonstrate that interning them would save a lot of memory. But would that resemble real-life usage patterns?

        So I suspect that some benchmark would be best, but indeed it's hard to revive my test cluster right now. Maybe I can still convince you by:

        • telling that String.intern() is proven to work well (I've already optimized several projects at Cloudera with its help, and there I could definitely quantify the effect of the changes - we can discuss all this offline if you would like)
        • attaching the results from my old benchmark showing how much memory is wasted due to duplicate strings in BlockLocation. I am attaching the full jxray report for one of the heap dumps that I obtained in this benchmark, and here are the most relevant excerpts:
        6. DUPLICATE STRINGS
        
        Total strings: 172,451  Unique strings: 52,360  Duplicate values: 16,158  Overhead: 14,291K (29.8%)
        
        Top duplicate strings:
            Ovhd         Num char[]s   Num objs   Value
        
          1,398K (2.9%)    12791       12791      "host-10-17-101-14.coe.cloudera.com"
          1,163K (2.4%)     9926        9926      "host-10-17-101-14.coe.cloudera.com:8020"
            809K (1.7%)        6           6      "hdfs://host-10-17-101-14.coe.cloudera.com:8020/tmp/misha/misha-table-partition-1,hdf ...[length 82892]"
            465K (1.0%)     9923        9923      "hdfs"
            ....
        
        7. REFERENCE CHAINS FOR DUPLICATE STRINGS
        
          595K (1.2%), 5088 dup strings (4 unique), 5088 dup backing arrays:
        1696 of "DS-aab6ab0b-0b11-489f-b209-ab2c6412934c", 1149 of "DS-d47bdaca-50c5-4475-ac08-7f07e10cd0b6", 1132 of "DS-bf6046e6-d5e9-4ac2-a1af-ff8a88ab9d85", 1111 of "DS-d2c5088c-bd69-4500-b981-502819c1307a"
             <-- String[] <-- org.apache.hadoop.fs.BlockLocation.storageIds <-- org.apache.hadoop.fs.BlockLocation[] <-- org.apache.hadoop.fs.LocatedFileStatus.locations <--  {j.u.ArrayList} <-- Java Local@fd414328 (j.u.ArrayList)
         
         556K (1.2%), 5088 dup strings (4 unique), 5088 dup backing arrays:
        1696 of "host-10-17-101-14.coe.cloudera.com", 1149 of "host-10-17-101-15.coe.cloudera.com", 1132 of "host-10-17-101-17.coe.cloudera.com", 1111 of "host-10-17-101-16.coe.cloudera.com"
             <-- String[] <-- org.apache.hadoop.fs.BlockLocation.hosts <-- org.apache.hadoop.fs.BlockLocation[] <-- org.apache.hadoop.fs.LocatedFileStatus.locations <--  {j.u.ArrayList} <-- Java Local@fd414328 (j.u.ArrayList)
        
          476K (1.0%), 5088 dup strings (4 unique), 5088 dup backing arrays:
        1696 of "/default/10.17.101.14:50010", 1149 of "/default/10.17.101.15:50010", 1132 of "/default/10.17.101.17:50010", 1111 of "/default/10.17.101.16:50010"
             <-- String[] <-- org.apache.hadoop.fs.BlockLocation.topologyPaths <-- org.apache.hadoop.fs.BlockLocation[] <-- org.apache.hadoop.fs.LocatedFileStatus.locations <--  {j.u.ArrayList} <-- Java Local@fd414328 (j.u.ArrayList)
        
          409K (0.9%), 3492 dup strings (4 unique), 3492 dup backing arrays:
        1164 of "DS-aab6ab0b-0b11-489f-b209-ab2c6412934c", 788 of "DS-d47bdaca-50c5-4475-ac08-7f07e10cd0b6", 770 of "DS-bf6046e6-d5e9-4ac2-a1af-ff8a88ab9d85", 770 of "DS-d2c5088c-bd69-4500-b981-502819c1307a"
             <-- String[] <-- org.apache.hadoop.fs.BlockLocation.storageIds <-- org.apache.hadoop.fs.BlockLocation[] <-- org.apache.hadoop.fs.LocatedFileStatus.locations <--  {j.u.ArrayList} <-- Java Local@fd67ae70 (j.u.ArrayList)
        
          397K (0.8%), 5088 dup strings (4 unique), 5088 dup backing arrays:
        1696 of "10.17.101.14:50010", 1149 of "10.17.101.15:50010", 1132 of "10.17.101.17:50010", 1111 of "10.17.101.16:50010"
             <-- String[] <-- org.apache.hadoop.fs.BlockLocation.names <-- org.apache.hadoop.fs.BlockLocation[] <-- org.apache.hadoop.fs.LocatedFileStatus.locations <--  {j.u.ArrayList} <-- Java Local@fd414328 (j.u.ArrayList)
        
          381K (0.8%), 3492 dup strings (4 unique), 3492 dup backing arrays:
        1164 of "host-10-17-101-14.coe.cloudera.com", 788 of "host-10-17-101-15.coe.cloudera.com", 770 of "host-10-17-101-17.coe.cloudera.com", 770 of "host-10-17-101-16.coe.cloudera.com"
             <-- String[] <-- org.apache.hadoop.fs.BlockLocation.hosts <-- org.apache.hadoop.fs.BlockLocation[] <-- org.apache.hadoop.fs.LocatedFileStatus.locations <--  {j.u.ArrayList} <-- Java Local@fd67ae70 (j.u.ArrayList)
        
        ....
        
        Show
        misha@cloudera.com Misha Dmitriev added a comment - Hi Andrew, I understand your concerns. Unit tests could be a good solution, but the problem is, to quantify the effect of a change like that one would need, in principle, to first run some code that uses BlockLocation unchanged and measure how much memory is consumed, then run the same code with BlockLocation that has interning and measure memory again. There is also a problem of how representative such a "pseudo-benchmark" would be, e.g. I can easily populate some data structure with very big strings and then demonstrate that interning them would save a lot of memory. But would that resemble real-life usage patterns? So I suspect that some benchmark would be best, but indeed it's hard to revive my test cluster right now. Maybe I can still convince you by: telling that String.intern() is proven to work well (I've already optimized several projects at Cloudera with its help, and there I could definitely quantify the effect of the changes - we can discuss all this offline if you would like) attaching the results from my old benchmark showing how much memory is wasted due to duplicate strings in BlockLocation. I am attaching the full jxray report for one of the heap dumps that I obtained in this benchmark, and here are the most relevant excerpts: 6. DUPLICATE STRINGS Total strings: 172,451 Unique strings: 52,360 Duplicate values: 16,158 Overhead: 14,291K (29.8%) Top duplicate strings: Ovhd Num char []s Num objs Value 1,398K (2.9%) 12791 12791 "host-10-17-101-14.coe.cloudera.com" 1,163K (2.4%) 9926 9926 "host-10-17-101-14.coe.cloudera.com:8020" 809K (1.7%) 6 6 "hdfs: //host-10-17-101-14.coe.cloudera.com:8020/tmp/misha/misha-table-partition-1,hdf ...[length 82892]" 465K (1.0%) 9923 9923 "hdfs" .... 7. REFERENCE CHAINS FOR DUPLICATE STRINGS 595K (1.2%), 5088 dup strings (4 unique), 5088 dup backing arrays: 1696 of "DS-aab6ab0b-0b11-489f-b209-ab2c6412934c" , 1149 of "DS-d47bdaca-50c5-4475-ac08-7f07e10cd0b6" , 1132 of "DS-bf6046e6-d5e9-4ac2-a1af-ff8a88ab9d85" , 1111 of "DS-d2c5088c-bd69-4500-b981-502819c1307a" <-- String [] <-- org.apache.hadoop.fs.BlockLocation.storageIds <-- org.apache.hadoop.fs.BlockLocation[] <-- org.apache.hadoop.fs.LocatedFileStatus.locations <-- {j.u.ArrayList} <-- Java Local@fd414328 (j.u.ArrayList) 556K (1.2%), 5088 dup strings (4 unique), 5088 dup backing arrays: 1696 of "host-10-17-101-14.coe.cloudera.com" , 1149 of "host-10-17-101-15.coe.cloudera.com" , 1132 of "host-10-17-101-17.coe.cloudera.com" , 1111 of "host-10-17-101-16.coe.cloudera.com" <-- String [] <-- org.apache.hadoop.fs.BlockLocation.hosts <-- org.apache.hadoop.fs.BlockLocation[] <-- org.apache.hadoop.fs.LocatedFileStatus.locations <-- {j.u.ArrayList} <-- Java Local@fd414328 (j.u.ArrayList) 476K (1.0%), 5088 dup strings (4 unique), 5088 dup backing arrays: 1696 of "/ default /10.17.101.14:50010" , 1149 of "/ default /10.17.101.15:50010" , 1132 of "/ default /10.17.101.17:50010" , 1111 of "/ default /10.17.101.16:50010" <-- String [] <-- org.apache.hadoop.fs.BlockLocation.topologyPaths <-- org.apache.hadoop.fs.BlockLocation[] <-- org.apache.hadoop.fs.LocatedFileStatus.locations <-- {j.u.ArrayList} <-- Java Local@fd414328 (j.u.ArrayList) 409K (0.9%), 3492 dup strings (4 unique), 3492 dup backing arrays: 1164 of "DS-aab6ab0b-0b11-489f-b209-ab2c6412934c" , 788 of "DS-d47bdaca-50c5-4475-ac08-7f07e10cd0b6" , 770 of "DS-bf6046e6-d5e9-4ac2-a1af-ff8a88ab9d85" , 770 of "DS-d2c5088c-bd69-4500-b981-502819c1307a" <-- String [] <-- org.apache.hadoop.fs.BlockLocation.storageIds <-- org.apache.hadoop.fs.BlockLocation[] <-- org.apache.hadoop.fs.LocatedFileStatus.locations <-- {j.u.ArrayList} <-- Java Local@fd67ae70 (j.u.ArrayList) 397K (0.8%), 5088 dup strings (4 unique), 5088 dup backing arrays: 1696 of "10.17.101.14:50010" , 1149 of "10.17.101.15:50010" , 1132 of "10.17.101.17:50010" , 1111 of "10.17.101.16:50010" <-- String [] <-- org.apache.hadoop.fs.BlockLocation.names <-- org.apache.hadoop.fs.BlockLocation[] <-- org.apache.hadoop.fs.LocatedFileStatus.locations <-- {j.u.ArrayList} <-- Java Local@fd414328 (j.u.ArrayList) 381K (0.8%), 3492 dup strings (4 unique), 3492 dup backing arrays: 1164 of "host-10-17-101-14.coe.cloudera.com" , 788 of "host-10-17-101-15.coe.cloudera.com" , 770 of "host-10-17-101-17.coe.cloudera.com" , 770 of "host-10-17-101-16.coe.cloudera.com" <-- String [] <-- org.apache.hadoop.fs.BlockLocation.hosts <-- org.apache.hadoop.fs.BlockLocation[] <-- org.apache.hadoop.fs.LocatedFileStatus.locations <-- {j.u.ArrayList} <-- Java Local@fd67ae70 (j.u.ArrayList) ....
        Hide
        andrew.wang Andrew Wang added a comment -

        Hi Misha, could you provide some indicative comparison numbers for this change? I get that it's safe, but it'd be good to document the expected improvement in heap usage. This should be doable with a unit test if you can't revive your test cluster.

        Show
        andrew.wang Andrew Wang added a comment - Hi Misha, could you provide some indicative comparison numbers for this change? I get that it's safe, but it'd be good to document the expected improvement in heap usage. This should be doable with a unit test if you can't revive your test cluster.
        Hide
        misha@cloudera.com Misha Dmitriev added a comment -

        Hi Manoj Govindassamy, in response to your question about String.intern() performance. Here is the best article on this that I found: http://java-performance.info/string-intern-in-java-6-7-8/

        One takeaway from it is that the internal hashtable used by the JVM for String.intern() is fixed-size and preallocated. So you essentially pay the memory price of it upfront whether you use it or not, but then it's free, unlike Guava. This implementation uses internal JVM pointers and other optimizations only possible inside the JVM, so it's faster and more economical than anything that can be written in Java.

        Show
        misha@cloudera.com Misha Dmitriev added a comment - Hi Manoj Govindassamy , in response to your question about String.intern() performance. Here is the best article on this that I found: http://java-performance.info/string-intern-in-java-6-7-8/ One takeaway from it is that the internal hashtable used by the JVM for String.intern() is fixed-size and preallocated. So you essentially pay the memory price of it upfront whether you use it or not, but then it's free, unlike Guava. This implementation uses internal JVM pointers and other optimizations only possible inside the JVM, so it's faster and more economical than anything that can be written in Java.
        Hide
        manojg Manoj Govindassamy added a comment -

        Misha Dmitriev,
        Thanks for filing this bug and providing a patch. Very useful one.

        String.intern() performs very well starting from JDK 7. I've found some articles explaining the progress that was made by the HotSpot JVM developers in this area, verified that with benchmarks myself.

        I am assuming the above benchmark was against the Guava's weak interner and String.intern() performed well.

        Patch looks good to me, +1 (non binding).

        Show
        manojg Manoj Govindassamy added a comment - Misha Dmitriev , Thanks for filing this bug and providing a patch. Very useful one. String.intern() performs very well starting from JDK 7. I've found some articles explaining the progress that was made by the HotSpot JVM developers in this area, verified that with benchmarks myself. I am assuming the above benchmark was against the Guava's weak interner and String.intern() performed well. Patch looks good to me, +1 (non binding).
        Hide
        misha@cloudera.com Misha Dmitriev added a comment -

        Hi Akira Ajisaka, thank you. Unfortunately, when I started working on this patch, I've discovered that an ad-hoc Hadoop cluster which I used to discover/reproduce this problem, is gone, and I cannot easily restore this setup. So it won't be straightforward to verify whether this worked. But given that the changes are pretty simple, there is low probability that it won't.

        Interestingly, just recently I've discovered the same problem when Impala used HMS and again loaded a large number of files. But it's in a heap dump received from a customer, so not really reproducible. Anyway, looks like this issue may be widespread enough to be worth attention.

        Show
        misha@cloudera.com Misha Dmitriev added a comment - Hi Akira Ajisaka , thank you. Unfortunately, when I started working on this patch, I've discovered that an ad-hoc Hadoop cluster which I used to discover/reproduce this problem, is gone, and I cannot easily restore this setup. So it won't be straightforward to verify whether this worked. But given that the changes are pretty simple, there is low probability that it won't. Interestingly, just recently I've discovered the same problem when Impala used HMS and again loaded a large number of files. But it's in a heap dump received from a customer, so not really reproducible. Anyway, looks like this issue may be widespread enough to be worth attention.
        Hide
        ajisakaa Akira Ajisaka added a comment -

        Nice catch and the patch looks good to me. Hi Misha Dmitriev, is there any benchmark result?

        Show
        ajisakaa Akira Ajisaka added a comment - Nice catch and the patch looks good to me. Hi Misha Dmitriev , is there any benchmark result?
        Hide
        misha@cloudera.com Misha Dmitriev added a comment -

        In response to the automatic comment above: the patch doesn't include any new tests because it doesn't change any functionality, only performance. Thus I believe to verify its correctness it's sufficient that the existing tests pass.

        Show
        misha@cloudera.com Misha Dmitriev added a comment - In response to the automatic comment above: the patch doesn't include any new tests because it doesn't change any functionality, only performance. Thus I believe to verify its correctness it's sufficient that the existing tests pass.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 15m 45s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 test4tests 0m 0s 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 mvninstall 14m 52s trunk passed
        +1 compile 14m 31s trunk passed
        +1 checkstyle 0m 38s trunk passed
        +1 mvnsite 1m 13s trunk passed
        +1 mvneclipse 0m 21s trunk passed
        -1 findbugs 1m 33s hadoop-common-project/hadoop-common in trunk has 19 extant Findbugs warnings.
        +1 javadoc 0m 53s trunk passed
        +1 mvninstall 0m 48s the patch passed
        +1 compile 15m 0s the patch passed
        +1 javac 15m 0s the patch passed
        -0 checkstyle 0m 46s hadoop-common-project/hadoop-common: The patch generated 4 new + 20 unchanged - 2 fixed = 24 total (was 22)
        +1 mvnsite 1m 12s the patch passed
        +1 mvneclipse 0m 22s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 54s the patch passed
        +1 javadoc 0m 56s the patch passed
        +1 unit 8m 23s hadoop-common in the patch passed.
        +1 asflicense 0m 37s The patch does not generate ASF License warnings.
        80m 59s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:14b5c93
        JIRA Issue HDFS-11383
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12868138/HDFS-11383.01.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 2253381a4ffd 3.13.0-107-generic #154-Ubuntu SMP Tue Dec 20 09:57:27 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / c48f297
        Default Java 1.8.0_131
        findbugs v3.1.0-RC1
        findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/19434/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-common-warnings.html
        checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/19434/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/19434/testReport/
        modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/19434/console
        Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 15m 45s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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 mvninstall 14m 52s trunk passed +1 compile 14m 31s trunk passed +1 checkstyle 0m 38s trunk passed +1 mvnsite 1m 13s trunk passed +1 mvneclipse 0m 21s trunk passed -1 findbugs 1m 33s hadoop-common-project/hadoop-common in trunk has 19 extant Findbugs warnings. +1 javadoc 0m 53s trunk passed +1 mvninstall 0m 48s the patch passed +1 compile 15m 0s the patch passed +1 javac 15m 0s the patch passed -0 checkstyle 0m 46s hadoop-common-project/hadoop-common: The patch generated 4 new + 20 unchanged - 2 fixed = 24 total (was 22) +1 mvnsite 1m 12s the patch passed +1 mvneclipse 0m 22s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 54s the patch passed +1 javadoc 0m 56s the patch passed +1 unit 8m 23s hadoop-common in the patch passed. +1 asflicense 0m 37s The patch does not generate ASF License warnings. 80m 59s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue HDFS-11383 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12868138/HDFS-11383.01.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 2253381a4ffd 3.13.0-107-generic #154-Ubuntu SMP Tue Dec 20 09:57:27 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / c48f297 Default Java 1.8.0_131 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/19434/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-common-warnings.html checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/19434/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/19434/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HDFS-Build/19434/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.

          People

          • Assignee:
            misha@cloudera.com Misha Dmitriev
            Reporter:
            misha@cloudera.com Misha Dmitriev
          • Votes:
            0 Vote for this issue
            Watchers:
            9 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development