From bd3dcf46e263b6e6aa3fca6a5d9936cc49e3280f Mon Sep 17 00:00:00 2001 From: Jing Zhao Date: Thu, 21 Jul 2016 11:14:39 -0700 Subject: [PATCH] HDFS-10653. Optimize conversion from path string to components. Contributed by Daryn Sharp. --- .../java/org/apache/hadoop/hdfs/DFSUtil.java | 9 ++++++ .../hadoop/hdfs/server/namenode/INode.java | 20 +++++-------- .../namenode/TestSnapshotPathINodes.java | 30 +++++++------------ 3 files changed, 26 insertions(+), 33 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSUtil.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSUtil.java index f1a6de7b0e..0ba80d9170 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSUtil.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSUtil.java @@ -319,6 +319,15 @@ public static String path2String(final Object path) { : path.toString(); } + /** + * Convert a UTF8 string to an array of byte arrays. + */ + public static byte[][] getPathComponents(String path) { + // avoid intermediate split to String[] + final byte[] bytes = string2Bytes(path); + return bytes2byteArray(bytes, bytes.length, (byte)Path.SEPARATOR_CHAR); + } + /** * Splits the array of bytes into array of arrays of bytes * on byte separator diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java index c8f36e137c..eb910d4b1f 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java @@ -727,18 +727,8 @@ public byte getStoragePolicyIDForQuota(byte parentStoragePolicyId) { */ @VisibleForTesting public static byte[][] getPathComponents(String path) { - return getPathComponents(getPathNames(path)); - } - - /** Convert strings to byte arrays for path components. */ - static byte[][] getPathComponents(String[] strings) { - if (strings.length == 0) { - return new byte[][]{null}; - } - byte[][] bytes = new byte[strings.length][]; - for (int i = 0; i < strings.length; i++) - bytes[i] = DFSUtil.string2Bytes(strings[i]); - return bytes; + checkAbsolutePath(path); + return DFSUtil.getPathComponents(path); } /** @@ -747,11 +737,15 @@ static byte[][] getPathComponents(String[] strings) { * @return array of path components. */ public static String[] getPathNames(String path) { + checkAbsolutePath(path); + return StringUtils.split(path, Path.SEPARATOR_CHAR); + } + + private static void checkAbsolutePath(final String path) { if (path == null || !path.startsWith(Path.SEPARATOR)) { throw new AssertionError("Absolute path required, but got '" + path + "'"); } - return StringUtils.split(path, Path.SEPARATOR_CHAR); } @Override diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestSnapshotPathINodes.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestSnapshotPathINodes.java index e416e00e1d..45c65ef8ce 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestSnapshotPathINodes.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestSnapshotPathINodes.java @@ -138,8 +138,7 @@ static void assertINodeFile(INode inode, Path path) { @Test (timeout=15000) public void testNonSnapshotPathINodes() throws Exception { // Get the inodes by resolving the path of a normal file - String[] names = INode.getPathNames(file1.toString()); - byte[][] components = INode.getPathComponents(names); + byte[][] components = INode.getPathComponents(file1.toString()); INodesInPath nodesInPath = INodesInPath.resolve(fsdir.rootDir, components, false); // The number of inodes should be equal to components.length @@ -175,8 +174,7 @@ public void testSnapshotPathINodes() throws Exception { // The path when accessing the snapshot file of file1 is // /TestSnapshot/sub1/.snapshot/s1/file1 String snapshotPath = sub1.toString() + "/.snapshot/s1/file1"; - String[] names = INode.getPathNames(snapshotPath); - byte[][] components = INode.getPathComponents(names); + byte[][] components = INode.getPathComponents(snapshotPath); INodesInPath nodesInPath = INodesInPath.resolve(fsdir.rootDir, components, false); // Length of inodes should be (components.length - 1), since we will ignore @@ -199,8 +197,7 @@ public void testSnapshotPathINodes() throws Exception { // Resolve the path "/TestSnapshot/sub1/.snapshot" String dotSnapshotPath = sub1.toString() + "/.snapshot"; - names = INode.getPathNames(dotSnapshotPath); - components = INode.getPathComponents(names); + components = INode.getPathComponents(dotSnapshotPath); nodesInPath = INodesInPath.resolve(fsdir.rootDir, components, false); // The number of INodes returned should still be components.length // since we put a null in the inode array for ".snapshot" @@ -246,8 +243,7 @@ public void testSnapshotPathINodesAfterDeletion() throws Exception { // Resolve the path for the snapshot file // /TestSnapshot/sub1/.snapshot/s2/file1 String snapshotPath = sub1.toString() + "/.snapshot/s2/file1"; - String[] names = INode.getPathNames(snapshotPath); - byte[][] components = INode.getPathComponents(names); + byte[][] components = INode.getPathComponents(snapshotPath); INodesInPath nodesInPath = INodesInPath.resolve(fsdir.rootDir, components, false); // Length of inodes should be (components.length - 1), since we will ignore @@ -264,8 +260,7 @@ public void testSnapshotPathINodesAfterDeletion() throws Exception { } // Check the INodes for path /TestSnapshot/sub1/file1 - String[] names = INode.getPathNames(file1.toString()); - byte[][] components = INode.getPathComponents(names); + byte[][] components = INode.getPathComponents(file1.toString()); INodesInPath nodesInPath = INodesInPath.resolve(fsdir.rootDir, components, false); // The length of inodes should be equal to components.length @@ -314,8 +309,7 @@ public void testSnapshotPathINodesWithAddedFile() throws Exception { { // Check the inodes for /TestSnapshot/sub1/.snapshot/s4/file3 String snapshotPath = sub1.toString() + "/.snapshot/s4/file3"; - String[] names = INode.getPathNames(snapshotPath); - byte[][] components = INode.getPathComponents(names); + byte[][] components = INode.getPathComponents(snapshotPath); INodesInPath nodesInPath = INodesInPath.resolve(fsdir.rootDir, components, false); // Length of inodes should be (components.length - 1), since we will ignore @@ -334,8 +328,7 @@ public void testSnapshotPathINodesWithAddedFile() throws Exception { } // Check the inodes for /TestSnapshot/sub1/file3 - String[] names = INode.getPathNames(file3.toString()); - byte[][] components = INode.getPathComponents(names); + byte[][] components = INode.getPathComponents(file3.toString()); INodesInPath nodesInPath = INodesInPath.resolve(fsdir.rootDir, components, false); // The number of inodes should be equal to components.length @@ -361,8 +354,7 @@ public void testSnapshotPathINodesWithAddedFile() throws Exception { @Test (timeout=15000) public void testSnapshotPathINodesAfterModification() throws Exception { // First check the INode for /TestSnapshot/sub1/file1 - String[] names = INode.getPathNames(file1.toString()); - byte[][] components = INode.getPathComponents(names); + byte[][] components = INode.getPathComponents(file1.toString()); INodesInPath nodesInPath = INodesInPath.resolve(fsdir.rootDir, components, false); // The number of inodes should be equal to components.length @@ -385,8 +377,7 @@ public void testSnapshotPathINodesAfterModification() throws Exception { // Check the INodes for snapshot of file1 String snapshotPath = sub1.toString() + "/.snapshot/s3/file1"; - names = INode.getPathNames(snapshotPath); - components = INode.getPathComponents(names); + components = INode.getPathComponents(snapshotPath); INodesInPath ssNodesInPath = INodesInPath.resolve(fsdir.rootDir, components, false); // Length of ssInodes should be (components.length - 1), since we will @@ -404,8 +395,7 @@ public void testSnapshotPathINodesAfterModification() throws Exception { snapshotFileNode.getModificationTime(ssNodesInPath.getPathSnapshotId())); // Check the INode for /TestSnapshot/sub1/file1 again - names = INode.getPathNames(file1.toString()); - components = INode.getPathComponents(names); + components = INode.getPathComponents(file1.toString()); INodesInPath newNodesInPath = INodesInPath.resolve(fsdir.rootDir, components, false); assertSnapshot(newNodesInPath, false, s3, -1);