From 4431cd7c582b95f4f78c65fb2bba9814ac93b799 Mon Sep 17 00:00:00 2001 From: Jan Henning Date: Mon, 4 Nov 2024 00:02:42 +0100 Subject: [PATCH 1/3] Speed up mapPath for files in large directories Opening a file from Windows Explorer results in multiple (possibly five or six) FIND_FIRST2 requests for the selected file. Each FIND_FIRST2 request results in a call to DiskInterface.mapPath(). In a large directory with thousands of files, each mapPath() has a considerable overhead (hundreds of ms), mainly caused by the calls to lastDir.list(), which have to list the whole directory before being able to proceed. For the directories at least that overhead is only incurred when actually needing to explicitly conduct a case-insensitive search, but for the file name currently lastDir.list() is always called unconditionally, which incurs the aforementioned large overhead. To speed up mapPath, I propose doing two things: - Switch the file names to same logic as the directory names, i.e. first we simply check whether the file exists, and only if that fails do we do the potentially more expensive iteration through the whole directory to check for a case-insensitive match. - Seeing how this is the Java*NIO*DiskDriver anyway, rewrite the directory search to actually use the new NIO APIs. --- .../smb/server/disk/JavaNIODiskDriver.java | 101 ++++++++---------- 1 file changed, 46 insertions(+), 55 deletions(-) diff --git a/src/main/java/org/filesys/smb/server/disk/JavaNIODiskDriver.java b/src/main/java/org/filesys/smb/server/disk/JavaNIODiskDriver.java index b25cb62..2c60bbf 100644 --- a/src/main/java/org/filesys/smb/server/disk/JavaNIODiskDriver.java +++ b/src/main/java/org/filesys/smb/server/disk/JavaNIODiskDriver.java @@ -27,6 +27,7 @@ import java.nio.file.DirectoryNotEmptyException; import java.nio.file.attribute.FileTime; import java.util.EnumSet; +import java.util.Iterator; import java.util.Random; import java.util.StringTokenizer; import java.util.concurrent.Executor; @@ -557,10 +558,10 @@ protected final String mapPath(String base, String path) int lastPos = pathStr.length(); idx = 0; - File lastDir = null; + Path lastDir = null; if (base != null && base.length() > 0) - lastDir = new File(base); - File curDir = null; + lastDir = Path.of(base); + Path curDir = null; while (idx < maxDir) { @@ -569,47 +570,46 @@ protected final String mapPath(String base, String path) pathStr.append(java.io.File.separator); // Check if the current path exists - curDir = new File(pathStr.toString()); + curDir = Path.of(pathStr.toString()); - if (curDir.exists() == false) { + if (!Files.exists(curDir)) { // Check if there is a previous directory to search if (lastDir == null) throw new PathNotFoundException(); // Search the current path for a matching directory, the case may be different - String[] fileList = lastDir.list(); - if (fileList == null || fileList.length == 0) - throw new PathNotFoundException(); - - int fidx = 0; - boolean foundPath = false; - - while (fidx < fileList.length && foundPath == false) { - - // Check if the current file name matches the required directory name - if (fileList[fidx].equalsIgnoreCase(dirs[idx])) { - - // Use the current directory name - pathStr.setLength(lastPos); - pathStr.append(fileList[fidx]); - pathStr.append(java.io.File.separator); - - // Check if the path is valid - curDir = new File(pathStr.toString()); - if (curDir.exists()) { - foundPath = true; - break; + try (DirectoryStream lastDirStream = Files.newDirectoryStream(lastDir)) { + Iterator lastDirIter = lastDirStream.iterator(); + + boolean foundPath = false; + + while (lastDirIter.hasNext() && foundPath == false) { + // Check if the current file name matches the required directory name + String candidateFile = lastDirIter.next().getFileName().toString(); + if (candidateFile.equalsIgnoreCase(dirs[idx])) { + + // Use the current directory name + pathStr.setLength(lastPos); + pathStr.append(candidateFile); + pathStr.append(java.io.File.separator); + + // Check if the path is valid + curDir = Path.of(pathStr.toString()); + if (Files.exists(curDir)) { + foundPath = true; + break; + } } } - // Update the file name index - fidx++; + // Check if we found the required directory + if (foundPath == false) + throw new PathNotFoundException(); } - - // Check if we found the required directory - if (foundPath == false) + catch (IOException ex) { throw new PathNotFoundException(); + } } // Set the last valid directory file @@ -626,37 +626,28 @@ protected final String mapPath(String base, String path) if (path.endsWith( FileName.DOS_SEPERATOR_STR) == false) { // Map the file name - String[] fileList = lastDir.list(); String fileName = dirs[dirs.length - 1]; + Path targetFile = Path.of(pathStr.toString(), fileName); - // Check if the file list is valid, if not then the path is not valid - if (fileList == null) - throw new FileNotFoundException(path); - - // Search for the required file - idx = 0; - boolean foundFile = false; - - while (idx < fileList.length && foundFile == false) { - if (fileList[idx].compareTo(fileName) == 0) - foundFile = true; - else - idx++; - } + // Search for the required file + boolean foundFile = Files.exists(targetFile); // Check if we found the file name, if not then do a case insensitive search if (foundFile == false) { + try (DirectoryStream lastDirStream = Files.newDirectoryStream(lastDir)) { + Iterator lastDirIter = lastDirStream.iterator(); - // Search again using a case insensitive search - idx = 0; + // Search again using a case insensitive search + while (lastDirIter.hasNext() && foundFile == false) { - while (idx < fileList.length && foundFile == false) { - if (fileList[idx].equalsIgnoreCase(fileName)) { - foundFile = true; - fileName = fileList[idx]; + String candidateFile = lastDirIter.next().getFileName().toString(); + if (candidateFile.equalsIgnoreCase(fileName)) { + foundFile = true; + fileName = candidateFile; + } } - else - idx++; + } catch (IOException ex) { + throw new PathNotFoundException(); } } From 7b228843cc85bdfd38e679befeb1495d107ebee4 Mon Sep 17 00:00:00 2001 From: Jan Henning Date: Mon, 4 Nov 2024 23:10:29 +0100 Subject: [PATCH 2/3] Skip mapping a filename that's in fact a wildcard search string No need to expensively query the file system for a file we know won't exist. --- .../smb/server/disk/JavaNIODiskDriver.java | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/filesys/smb/server/disk/JavaNIODiskDriver.java b/src/main/java/org/filesys/smb/server/disk/JavaNIODiskDriver.java index 2c60bbf..edb521e 100644 --- a/src/main/java/org/filesys/smb/server/disk/JavaNIODiskDriver.java +++ b/src/main/java/org/filesys/smb/server/disk/JavaNIODiskDriver.java @@ -627,27 +627,31 @@ protected final String mapPath(String base, String path) // Map the file name String fileName = dirs[dirs.length - 1]; - Path targetFile = Path.of(pathStr.toString(), fileName); - // Search for the required file - boolean foundFile = Files.exists(targetFile); + // If it's in fact a wildcard search, we can skip attempting to map a real file + if (!WildCard.containsWildcards(fileName)) { + Path targetFile = Path.of(pathStr.toString(), fileName); - // Check if we found the file name, if not then do a case insensitive search - if (foundFile == false) { - try (DirectoryStream lastDirStream = Files.newDirectoryStream(lastDir)) { - Iterator lastDirIter = lastDirStream.iterator(); + // Search for the required file + boolean foundFile = Files.exists(targetFile); - // Search again using a case insensitive search - while (lastDirIter.hasNext() && foundFile == false) { + // Check if we found the file name, if not then do a case insensitive search + if (foundFile == false) { + try (DirectoryStream lastDirStream = Files.newDirectoryStream(lastDir)) { + Iterator lastDirIter = lastDirStream.iterator(); - String candidateFile = lastDirIter.next().getFileName().toString(); - if (candidateFile.equalsIgnoreCase(fileName)) { - foundFile = true; - fileName = candidateFile; + // Search again using a case insensitive search + while (lastDirIter.hasNext() && foundFile == false) { + + String candidateFile = lastDirIter.next().getFileName().toString(); + if (candidateFile.equalsIgnoreCase(fileName)) { + foundFile = true; + fileName = candidateFile; + } } + } catch (IOException ex) { + throw new PathNotFoundException(); } - } catch (IOException ex) { - throw new PathNotFoundException(); } } From 84f587af9ebc751e848a355de1b7154fd0c31c4a Mon Sep 17 00:00:00 2001 From: Jan Henning Date: Sun, 21 Sep 2025 21:06:21 +0200 Subject: [PATCH 3/3] Simplify path mapping when the underlying disk is known to be case-insensitive If the underlying disk device is known to be case-insensitive, a nonexistent file really is nonexistent, so we can skip trying to traverse the whole directory in search of a possibly matching file with a different case. --- .../smb/server/disk/JavaNIODeviceContext.java | 17 ++++++ .../smb/server/disk/JavaNIODiskDriver.java | 60 +++++++++++-------- 2 files changed, 51 insertions(+), 26 deletions(-) diff --git a/src/main/java/org/filesys/smb/server/disk/JavaNIODeviceContext.java b/src/main/java/org/filesys/smb/server/disk/JavaNIODeviceContext.java index 7d4d049..4439e29 100644 --- a/src/main/java/org/filesys/smb/server/disk/JavaNIODeviceContext.java +++ b/src/main/java/org/filesys/smb/server/disk/JavaNIODeviceContext.java @@ -50,6 +50,9 @@ public class JavaNIODeviceContext extends DiskDeviceContext { // Large file size, require special processing for deletes/truncates private long m_largeFileSize = DefaultLargeFileSize; + // Whether the underlying file system can be assumed to be case-insensitive + private boolean m_caseInsensitiveDisk = false; + /** * Class constructor * @@ -144,6 +147,10 @@ else if ( m_trashDir.isFile()) m_trashDir = trashDir; } + // Check if we can assume a case-insensitive file system + if (args.getChild("DiskIsCaseInsensitive") != null) + m_caseInsensitiveDisk = true; + // Check if debug output is enabled if ( args.getChild( "Debug") != null) setDebug( true); @@ -195,4 +202,14 @@ protected final File getTrashFolder() { protected final long getLargeFileSize() { return m_largeFileSize; } + + /** + * Whether the underlying disk is assumed to be case-insensitive, allowing + * certain performance optimizations to be made + * + * @return boolean + */ + protected final boolean getDiskIsCaseInsensitive() { + return m_caseInsensitiveDisk; + } } diff --git a/src/main/java/org/filesys/smb/server/disk/JavaNIODiskDriver.java b/src/main/java/org/filesys/smb/server/disk/JavaNIODiskDriver.java index edb521e..e3f0784 100644 --- a/src/main/java/org/filesys/smb/server/disk/JavaNIODiskDriver.java +++ b/src/main/java/org/filesys/smb/server/disk/JavaNIODiskDriver.java @@ -222,8 +222,8 @@ public NetworkFile createFile(SrvSession sess, TreeConnection tree, FileOpenPara throws java.io.IOException { // Get the full path for the new file - DeviceContext ctx = tree.getContext(); - Path newPath = Paths.get( mapPath( ctx.getDeviceName(), params.getPath())); + JavaNIODeviceContext ctx = (JavaNIODeviceContext) tree.getContext(); + Path newPath = Paths.get(mapPath(ctx.getDeviceName(), params.getPath(), ctx.getDiskIsCaseInsensitive())); // Check if the file already exists if ( Files.exists( newPath, LinkOption.NOFOLLOW_LINKS)) @@ -268,7 +268,7 @@ public void deleteDirectory(SrvSession sess, TreeConnection tree, String dir) throws java.io.IOException { // Get the full path for the directory - DeviceContext ctx = tree.getContext(); + JavaNIODeviceContext ctx = (JavaNIODeviceContext) tree.getContext(); Path dirPath = Paths.get( FileName.buildPath(ctx.getDeviceName(), dir, null, java.io.File.separatorChar)); // Check if the directory exists, and it is a directory @@ -287,7 +287,7 @@ public void deleteDirectory(SrvSession sess, TreeConnection tree, String dir) else if ( Files.exists( dirPath) == false) { // Map the path to a real path - String mappedPath = mapPath(ctx.getDeviceName(), dir); + String mappedPath = mapPath(ctx.getDeviceName(), dir, ctx.getDiskIsCaseInsensitive()); if (mappedPath != null) { @@ -391,7 +391,7 @@ public void run() { public FileStatus fileExists(SrvSession sess, TreeConnection tree, String name) { // Get the full path for the file - DeviceContext ctx = tree.getContext(); + JavaNIODeviceContext ctx = (JavaNIODeviceContext) tree.getContext(); Path filePath = Paths.get( FileName.buildPath(ctx.getDeviceName(), name, null, java.io.File.separatorChar)); if ( Files.exists( filePath, LinkOption.NOFOLLOW_LINKS)) { @@ -405,7 +405,7 @@ public FileStatus fileExists(SrvSession sess, TreeConnection tree, String name) // Map the path, and re-check try { - String mappedPath = mapPath(ctx.getDeviceName(), name); + String mappedPath = mapPath(ctx.getDeviceName(), name, ctx.getDiskIsCaseInsensitive()); if ( mappedPath != null) { filePath = Paths.get( mappedPath); @@ -455,7 +455,7 @@ public FileInfo getFileInformation(SrvSession sess, TreeConnection tree, String throws java.io.IOException { // Get the full path for the file/directory - DeviceContext ctx = tree.getContext(); + JavaNIODeviceContext ctx = (JavaNIODeviceContext) tree.getContext(); String path = FileName.buildPath(ctx.getDeviceName(), name, null, java.io.File.separatorChar); // Build the file information for the file/directory @@ -465,7 +465,7 @@ public FileInfo getFileInformation(SrvSession sess, TreeConnection tree, String return info; // Try and map the path to a real path - String mappedPath = mapPath(ctx.getDeviceName(), name); + String mappedPath = mapPath(ctx.getDeviceName(), name, ctx.getDiskIsCaseInsensitive()); if (mappedPath != null) return buildFileInformation(mappedPath, name); @@ -495,30 +495,33 @@ public boolean isReadOnly(SrvSession sess, DeviceContext ctx) } /** - * Map the input path to a real path, this may require changing the case of various parts of the - * path. + * Map the input path to a real path, this may require changing the case of + * various parts of the path. * - * @param path Share relative path + * @param path Share relative path + * @param caseInsensitive boolean * @return Real path on the local filesystem * @exception FileNotFoundException The path could not be mapped to a real path. * @exception PathNotFoundException Part of the path is not valid */ - protected final String mapPath(String path) + protected final String mapPath(String path, boolean caseInsensitive) throws java.io.FileNotFoundException, PathNotFoundException { - return mapPath("", path); + return mapPath("", path, caseInsensitive); } /** - * Map the input path to a real path, this may require changing the case of various parts of the - * path. The base path is not checked, it is assumed to exist. + * Map the input path to a real path, this may require changing the case of + * various parts of the path. The base path is not checked, it is assumed to + * exist. * - * @param base String - * @param path String + * @param base String + * @param path String + * @param caseInsensitive boolean * @return String * @exception FileNotFoundException The path could not be mapped to a real path. * @exception PathNotFoundException Part of the path is not valid */ - protected final String mapPath(String base, String path) + protected final String mapPath(String base, String path, boolean caseInsensitive) throws java.io.FileNotFoundException, PathNotFoundException { // Split the path string into seperate directory components @@ -575,7 +578,9 @@ protected final String mapPath(String base, String path) if (!Files.exists(curDir)) { // Check if there is a previous directory to search - if (lastDir == null) + // (Unless the disk is case-insensitive, because in that case the file really + // doesn't exist) + if (lastDir == null || caseInsensitive) throw new PathNotFoundException(); // Search the current path for a matching directory, the case may be different @@ -636,7 +641,9 @@ protected final String mapPath(String base, String path) boolean foundFile = Files.exists(targetFile); // Check if we found the file name, if not then do a case insensitive search - if (foundFile == false) { + // (Unless the disk is already assumed to be case-insensitive, in which case + // skip this step) + if (foundFile == false && !caseInsensitive) { try (DirectoryStream lastDirStream = Files.newDirectoryStream(lastDir)) { Iterator lastDirIter = lastDirStream.iterator(); @@ -684,13 +691,13 @@ public NetworkFile openFile(SrvSession sess, TreeConnection tree, FileOpenParams throws java.io.IOException { // Create a Java network file - DeviceContext ctx = tree.getContext(); + JavaNIODeviceContext ctx = (JavaNIODeviceContext) tree.getContext(); Path filePath = Paths.get( FileName.buildPath(ctx.getDeviceName(), params.getPath(), null, java.io.File.separatorChar)); if ( Files.exists( filePath) == false) { // Try and map the file name string to a local path - String mappedPath = mapPath(ctx.getDeviceName(), params.getPath()); + String mappedPath = mapPath(ctx.getDeviceName(), params.getPath(), ctx.getDiskIsCaseInsensitive()); if (mappedPath == null) throw new java.io.FileNotFoundException(filePath.toString()); @@ -845,8 +852,8 @@ public void setFileInformation(SrvSession sess, TreeConnection tree, String name if (info.hasSetFlag(FileInfo.SetModifyDate)) { // Build the path to the file - DeviceContext ctx = tree.getContext(); - Path filePath = Paths.get( mapPath( ctx.getDeviceName(), name)); + JavaNIODeviceContext ctx = (JavaNIODeviceContext) tree.getContext(); + Path filePath = Paths.get(mapPath(ctx.getDeviceName(), name, ctx.getDiskIsCaseInsensitive())); // Update the file/folder modify date/time Files.setLastModifiedTime( filePath, FileTime.fromMillis( info.getModifyDateTime())); @@ -868,13 +875,14 @@ public SearchContext startSearch(SrvSession sess, TreeConnection tree, String se throws java.io.FileNotFoundException { // Create the full search path string - String path = FileName.buildPath(tree.getContext().getDeviceName(), null, searchPath, File.separatorChar); + JavaNIODeviceContext diskCtx = (JavaNIODeviceContext) tree.getContext(); + String path = FileName.buildPath(diskCtx.getDeviceName(), null, searchPath, File.separatorChar); JavaNIOSearchContext ctx = null; try { // Map the path, this may require changing the case on some or all path components - path = mapPath(path); + path = mapPath(path, diskCtx.getDiskIsCaseInsensitive()); // Split the search path to get the share relative path String[] paths = FileName.splitPath(path, File.separatorChar);