Details
Description
Converting a path String to a byte[][] currently requires an unnecessary intermediate conversion from String to String[]. Removing this will reduce excessive object allocation and byte copying.
Issue Links
- is required by
-
HDFS-10655
Fix path related byte array conversion bugs
-
- Resolved
-
Activity
- All
- Comments
- Work Log
- History
- Activity
- Transitions
May I have a review please? It's going to become harder for me to post more patches until the outstanding ones under the umbrella are integrated.
The patch looks good to me. +1.
I've committed this to trunk and branch-2.
Thanks Daryn for the patch and Jing for the review. I quickly tried and there's only a minor conflict to 2.6. I'll try to backport now.
SUCCESS: Integrated in Hadoop-trunk-Commit #10130 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10130/)
HDFS-10653. Optimize conversion from path string to components. (jing9: rev bd3dcf46e263b6e6aa3fca6a5d9936cc49e3280f)
- hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestSnapshotPathINodes.java
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSUtil.java
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java
Attaching branch-2.6 patch to trigger Jenkins and also ping Daryn Sharp to review the following conflict resolution:
// FSNamesystem#mkdirsRecursively String[] names = INode.getPathNames(src); byte[][] components = INode.getPathComponents(src); // used to be getPathComponents(names)
Thanks Jing and Zhe! Zhe, +1 looks fine but you may want to try and eliminate "String[] names = INode.getPathNames(src)".
Sorry, have to reopen the issue otherwise Jenkins won't run.
 -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 appears to include 1 new or modified test files. |
| +1 | mvninstall | 29m 15s | branch-2.6 passed |
| -1 | compile | 1m 6s | hadoop-hdfs in branch-2.6 failed with JDK v1.8.0_91. |
| -1 | compile | 0m 57s | hadoop-hdfs in branch-2.6 failed with JDK v1.7.0_101. |
| +1 | checkstyle | 0m 29s | branch-2.6 passed |
| +1 | mvnsite | 1m 19s | branch-2.6 passed |
| +1 | mvneclipse | 0m 21s | branch-2.6 passed |
| -1 | findbugs | 3m 32s | hadoop-hdfs-project/hadoop-hdfs in branch-2.6 has 273 extant Findbugs warnings. |
| +1 | javadoc | 1m 9s | branch-2.6 passed with JDK v1.8.0_91 |
| +1 | javadoc | 1m 51s | branch-2.6 passed with JDK v1.7.0_101 |
| +1 | mvninstall | 1m 1s | the patch passed |
| -1 | compile | 0m 49s | hadoop-hdfs in the patch failed with JDK v1.8.0_91. |
| -1 | javac | 0m 49s | hadoop-hdfs in the patch failed with JDK v1.8.0_91. |
| -1 | compile | 0m 48s | hadoop-hdfs in the patch failed with JDK v1.7.0_101. |
| -1 | javac | 0m 48s | hadoop-hdfs in the patch failed with JDK v1.7.0_101. |
| +1 | checkstyle | 0m 21s | the patch passed |
| +1 | mvnsite | 1m 1s | the patch passed |
| +1 | mvneclipse | 0m 15s | the patch passed |
| -1 | whitespace | 0m 0s | The patch has 1634 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply |
| -1 | whitespace | 0m 38s | The patch 77 line(s) with tabs. |
| +1 | findbugs | 3m 11s | the patch passed |
| +1 | javadoc | 1m 11s | the patch passed with JDK v1.8.0_91 |
| +1 | javadoc | 1m 54s | the patch passed with JDK v1.7.0_101 |
| -1 | unit | 0m 48s | hadoop-hdfs in the patch failed with JDK v1.7.0_101. |
| -1 | asflicense | 0m 37s | The patch generated 75 ASF License warnings. |
| 56m 34s |
This message was automatically generated.
Thanks for the suggestion Daryn Sharp. Attaching new 2.6 patch to avoid resolving twice.
| -1 overall |
| Vote | Subsystem | Runtime | Comment |
|---|---|---|---|
| 0 | reexec | 0m 31s | Docker mode activated. |
| +1 | @author | 0m 0s | The patch does not contain any @author tags. |
| +1 | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
| +1 | mvninstall | 5m 50s | branch-2.6 passed |
| -1 | compile | 0m 52s | hadoop-hdfs in branch-2.6 failed with JDK v1.8.0_91. |
| -1 | compile | 0m 46s | hadoop-hdfs in branch-2.6 failed with JDK v1.7.0_101. |
| +1 | checkstyle | 0m 22s | branch-2.6 passed |
| +1 | mvnsite | 1m 0s | branch-2.6 passed |
| +1 | mvneclipse | 0m 15s | branch-2.6 passed |
| -1 | findbugs | 2m 58s | hadoop-hdfs-project/hadoop-hdfs in branch-2.6 has 273 extant Findbugs warnings. |
| +1 | javadoc | 1m 11s | branch-2.6 passed with JDK v1.8.0_91 |
| +1 | javadoc | 1m 50s | branch-2.6 passed with JDK v1.7.0_101 |
| +1 | mvninstall | 0m 58s | the patch passed |
| -1 | compile | 0m 49s | hadoop-hdfs in the patch failed with JDK v1.8.0_91. |
| -1 | javac | 0m 49s | hadoop-hdfs in the patch failed with JDK v1.8.0_91. |
| -1 | compile | 0m 45s | hadoop-hdfs in the patch failed with JDK v1.7.0_101. |
| -1 | javac | 0m 45s | hadoop-hdfs in the patch failed with JDK v1.7.0_101. |
| +1 | checkstyle | 0m 20s | the patch passed |
| +1 | mvnsite | 0m 57s | the patch passed |
| +1 | mvneclipse | 0m 14s | the patch passed |
| -1 | whitespace | 0m 1s | The patch has 1953 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply |
| -1 | whitespace | 0m 43s | The patch 77 line(s) with tabs. |
| -1 | findbugs | 3m 20s | hadoop-hdfs-project/hadoop-hdfs generated 1 new + 273 unchanged - 0 fixed = 274 total (was 273) |
| +1 | javadoc | 1m 6s | the patch passed with JDK v1.8.0_91 |
| +1 | javadoc | 1m 48s | the patch passed with JDK v1.7.0_101 |
| -1 | unit | 0m 48s | hadoop-hdfs in the patch failed with JDK v1.7.0_101. |
| -1 | asflicense | 0m 34s | The patch generated 75 ASF License warnings. |
| 31m 22s |
| Reason | Tests |
|---|---|
| FindBugs | module:hadoop-hdfs-project/hadoop-hdfs |
| Invocation of toString on an unnamed array in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.mkdirsRecursively(String, PermissionStatus, boolean, long) At FSNamesystem.java:unnamed array in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.mkdirsRecursively(String, PermissionStatus, boolean, long) At FSNamesystem.java:[line 4346] |
This message was automatically generated.
Updating patch to address Findbugs issue.
Allen Wittenauer Do you have a clue why test results page is not available for the branch-2.6 patches? Thanks much.
| -1 overall |
| Vote | Subsystem | Runtime | Comment |
|---|---|---|---|
| 0 | reexec | 12m 35s | Docker mode activated. |
| +1 | @author | 0m 0s | The patch does not contain any @author tags. |
| +1 | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
| +1 | mvninstall | 21m 38s | branch-2.6 passed |
| -1 | compile | 0m 49s | hadoop-hdfs in branch-2.6 failed with JDK v1.8.0_101. |
| -1 | compile | 0m 51s | hadoop-hdfs in branch-2.6 failed with JDK v1.7.0_101. |
| +1 | checkstyle | 0m 28s | branch-2.6 passed |
| +1 | mvnsite | 1m 4s | branch-2.6 passed |
| +1 | mvneclipse | 0m 21s | branch-2.6 passed |
| -1 | findbugs | 3m 35s | hadoop-hdfs-project/hadoop-hdfs in branch-2.6 has 272 extant Findbugs warnings. |
| +1 | javadoc | 1m 16s | branch-2.6 passed with JDK v1.8.0_101 |
| +1 | javadoc | 2m 7s | branch-2.6 passed with JDK v1.7.0_101 |
| +1 | mvninstall | 1m 6s | the patch passed |
| -1 | compile | 0m 44s | hadoop-hdfs in the patch failed with JDK v1.8.0_101. |
| -1 | javac | 0m 44s | hadoop-hdfs in the patch failed with JDK v1.8.0_101. |
| -1 | compile | 0m 46s | hadoop-hdfs in the patch failed with JDK v1.7.0_101. |
| -1 | javac | 0m 46s | hadoop-hdfs in the patch failed with JDK v1.7.0_101. |
| +1 | checkstyle | 0m 20s | the patch passed |
| +1 | mvnsite | 0m 56s | the patch passed |
| +1 | mvneclipse | 0m 14s | the patch passed |
| -1 | whitespace | 0m 0s | The patch has 1952 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply |
| -1 | whitespace | 0m 46s | The patch 77 line(s) with tabs. |
| -1 | findbugs | 3m 14s | hadoop-hdfs-project/hadoop-hdfs generated 1 new + 272 unchanged - 0 fixed = 273 total (was 272) |
| +1 | javadoc | 1m 8s | the patch passed with JDK v1.8.0_101 |
| +1 | javadoc | 1m 55s | the patch passed with JDK v1.7.0_101 |
| -1 | unit | 0m 47s | hadoop-hdfs in the patch failed with JDK v1.7.0_101. |
| -1 | asflicense | 0m 42s | The patch generated 75 ASF License warnings. |
| 60m 57s |
| Reason | Tests |
|---|---|
| FindBugs | module:hadoop-hdfs-project/hadoop-hdfs |
| Invocation of toString on an unnamed array in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.mkdirsRecursively(String, PermissionStatus, boolean, long) At FSNamesystem.java:unnamed array in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.mkdirsRecursively(String, PermissionStatus, boolean, long) At FSNamesystem.java:[line 4346] |
This message was automatically generated.
Do you have a clue why test results page is not available for the branch-2.6 patches?
TL;DR: branch-2.6 support was removed from precommit.
Longer:
/usr/lib/jvm/java-8-oracle:1.8.0_101 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_101
As mentioned months and months ago on common-dev, JDK6 was jettisoned in favor of JDK8 until Apache Yetus can support different settings per branch. In addition, I have no idea what JDKs are in the branch-2.6 dockerfile, but given that jdk8 is available, I'm guessing someone blindly backported a trunk patch without considering the consequences.
| -1 overall |
| Vote | Subsystem | Runtime | Comment |
|---|---|---|---|
| 0 | reexec | 0m 23s | Docker mode activated. |
| +1 | @author | 0m 0s | The patch does not contain any @author tags. |
| +1 | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
| +1 | mvninstall | 14m 14s | branch-2.6 passed |
| -1 | compile | 0m 49s | hadoop-hdfs in branch-2.6 failed with JDK v1.8.0_101. |
| -1 | compile | 0m 46s | hadoop-hdfs in branch-2.6 failed with JDK v1.7.0_101. |
| +1 | checkstyle | 0m 23s | branch-2.6 passed |
| +1 | mvnsite | 0m 58s | branch-2.6 passed |
| +1 | mvneclipse | 0m 17s | branch-2.6 passed |
| -1 | findbugs | 2m 56s | hadoop-hdfs-project/hadoop-hdfs in branch-2.6 has 273 extant Findbugs warnings. |
| +1 | javadoc | 1m 15s | branch-2.6 passed with JDK v1.8.0_101 |
| +1 | javadoc | 1m 56s | branch-2.6 passed with JDK v1.7.0_101 |
| +1 | mvninstall | 0m 55s | the patch passed |
| -1 | compile | 0m 48s | hadoop-hdfs in the patch failed with JDK v1.8.0_101. |
| -1 | javac | 0m 48s | hadoop-hdfs in the patch failed with JDK v1.8.0_101. |
| -1 | compile | 0m 42s | hadoop-hdfs in the patch failed with JDK v1.7.0_101. |
| -1 | javac | 0m 42s | hadoop-hdfs in the patch failed with JDK v1.7.0_101. |
| +1 | checkstyle | 0m 19s | the patch passed |
| +1 | mvnsite | 0m 54s | the patch passed |
| +1 | mvneclipse | 0m 14s | the patch passed |
| -1 | whitespace | 0m 0s | The patch has 2292 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply |
| -1 | whitespace | 0m 56s | The patch 77 line(s) with tabs. |
| +1 | findbugs | 3m 7s | the patch passed |
| +1 | javadoc | 1m 7s | the patch passed with JDK v1.8.0_101 |
| +1 | javadoc | 1m 49s | the patch passed with JDK v1.7.0_101 |
| -1 | unit | 0m 43s | hadoop-hdfs in the patch failed with JDK v1.7.0_101. |
| -1 | asflicense | 0m 35s | The patch generated 75 ASF License warnings. |
| 39m 34s |
This message was automatically generated.
| -1 overall |
| Vote | Subsystem | Runtime | Comment |
|---|---|---|---|
| 0 | reexec | 0m 21s | Docker mode activated. |
| +1 | @author | 0m 0s | The patch does not contain any @author tags. |
| +1 | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
| +1 | mvninstall | 6m 10s | branch-2.6 passed |
| -1 | compile | 0m 50s | hadoop-hdfs in branch-2.6 failed with JDK v1.8.0_101. |
| -1 | compile | 0m 50s | hadoop-hdfs in branch-2.6 failed with JDK v1.7.0_101. |
| +1 | checkstyle | 0m 22s | branch-2.6 passed |
| +1 | mvnsite | 1m 3s | branch-2.6 passed |
| +1 | mvneclipse | 0m 16s | branch-2.6 passed |
| -1 | findbugs | 3m 23s | hadoop-hdfs-project/hadoop-hdfs in branch-2.6 has 272 extant Findbugs warnings. |
| +1 | javadoc | 1m 12s | branch-2.6 passed with JDK v1.8.0_101 |
| +1 | javadoc | 1m 58s | branch-2.6 passed with JDK v1.7.0_101 |
| +1 | mvninstall | 1m 0s | the patch passed |
| -1 | compile | 0m 45s | hadoop-hdfs in the patch failed with JDK v1.8.0_101. |
| -1 | javac | 0m 45s | hadoop-hdfs in the patch failed with JDK v1.8.0_101. |
| -1 | compile | 0m 50s | hadoop-hdfs in the patch failed with JDK v1.7.0_101. |
| -1 | javac | 0m 50s | hadoop-hdfs in the patch failed with JDK v1.7.0_101. |
| +1 | checkstyle | 0m 22s | the patch passed |
| +1 | mvnsite | 1m 3s | the patch passed |
| +1 | mvneclipse | 0m 16s | the patch passed |
| -1 | whitespace | 0m 0s | The patch has 2618 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply |
| -1 | whitespace | 1m 5s | The patch 77 line(s) with tabs. |
| +1 | findbugs | 3m 36s | the patch passed |
| +1 | javadoc | 1m 18s | the patch passed with JDK v1.8.0_101 |
| +1 | javadoc | 2m 4s | the patch passed with JDK v1.7.0_101 |
| -1 | unit | 0m 51s | hadoop-hdfs in the patch failed with JDK v1.7.0_101. |
| -1 | asflicense | 0m 33s | The patch generated 75 ASF License warnings. |
| 33m 52s |
This message was automatically generated.
Arrays.toString(components[i]) is going to place square brackets, ie. "[component]" around the name. DFSUtil.bytes2String(int,int) will let you avoid using a StringBuilder entirely - which has been part of the goal of my patches. +1 after you change it. Bonus points for not constructing the path unless required but not that concerned about 2.6.
Thanks Daryn Sharp for the suggestion. Good point. Updating patch to address.
Allen Wittenauer Thanks for explaining pre-commit. I'll run test-patch locally for 2.6 patches then.
Will commit v03 patch after local test-patch run.
Local test-patch shows 2 failures but those tests fail even without the patch. I'll create a JIRA to address.
Just committed v03 branch-2.6 patch. Thanks Allen and Daryn for the review and comments.
HDFS-10653This message was automatically generated.