Skip to content

Commit 661885a

Browse files
gukoffKonstantin Gukov
andauthored
Make File.Move(..., ..., overwrite: true) more efficient. (#47118)
The current implementation has an inefficiency. When moving a file across volumes, and when the target file doesn't exist, the "rename" syscall will be called twice (unsuccesfully) instead of once. By branching on "overwrite" from the beginning, we avoid calling the first "rename" in vain. Co-authored-by: Konstantin Gukov <kgukov@microsoft.com>
1 parent 73da00f commit 661885a

File tree

1 file changed

+23
-22
lines changed

1 file changed

+23
-22
lines changed

src/libraries/System.IO.FileSystem/src/System/IO/FileSystem.Unix.cs

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -139,27 +139,6 @@ public static void MoveFile(string sourceFullPath, string destFullPath)
139139

140140
public static void MoveFile(string sourceFullPath, string destFullPath, bool overwrite)
141141
{
142-
// The desired behavior for Move(source, dest) is to not overwrite the destination file
143-
// if it exists. Since rename(source, dest) will replace the file at 'dest' if it exists,
144-
// link/unlink are used instead. Rename is more efficient than link/unlink on file systems
145-
// where hard links are not supported (such as FAT). Therefore, given that source file exists,
146-
// rename is used in 2 cases: when dest file does not exist or when source path and dest
147-
// path refer to the same file (on the same device). This is important for case-insensitive
148-
// file systems (e.g. renaming a file in a way that just changes casing), so that we support
149-
// changing the casing in the naming of the file. If this fails in any way (e.g. source file
150-
// doesn't exist, dest file doesn't exist, rename fails, etc.), we just fall back to trying the
151-
// link/unlink approach and generating any exceptional messages from there as necessary.
152-
Interop.Sys.FileStatus sourceStat, destStat;
153-
if (Interop.Sys.LStat(sourceFullPath, out sourceStat) == 0 && // source file exists
154-
(Interop.Sys.LStat(destFullPath, out destStat) != 0 || // dest file does not exist
155-
(sourceStat.Dev == destStat.Dev && // source and dest are on the same device
156-
sourceStat.Ino == destStat.Ino)) && // source and dest are the same file on that device
157-
Interop.Sys.Rename(sourceFullPath, destFullPath) == 0) // try the rename
158-
{
159-
// Renamed successfully.
160-
return;
161-
}
162-
163142
// If overwrite is allowed then just call rename
164143
if (overwrite)
165144
{
@@ -179,7 +158,7 @@ public static void MoveFile(string sourceFullPath, string destFullPath, bool ove
179158
// when the error occurs and our checks, but it's the best we can do, and the
180159
// worst case in such a race condition (which could occur if the file system is
181160
// being manipulated concurrently with these checks) is that we throw a
182-
// FileNotFoundException instead of DirectoryNotFoundexception.
161+
// FileNotFoundException instead of DirectoryNotFoundException.
183162
throw Interop.GetExceptionForIoErrno(errorInfo, destFullPath,
184163
isDirectory: errorInfo.Error == Interop.Error.ENOENT && !Directory.Exists(Path.GetDirectoryName(destFullPath)) // The parent directory of destFile can't be found
185164
);
@@ -190,6 +169,28 @@ public static void MoveFile(string sourceFullPath, string destFullPath, bool ove
190169
return;
191170
}
192171

172+
// The desired behavior for Move(source, dest) is to not overwrite the destination file
173+
// if it exists. Since rename(source, dest) will replace the file at 'dest' if it exists,
174+
// link/unlink are used instead. Rename is more efficient than link/unlink on file systems
175+
// where hard links are not supported (such as FAT). Therefore, given that source file exists,
176+
// rename is used in 2 cases: when dest file does not exist or when source path and dest
177+
// path refer to the same file (on the same device). This is important for case-insensitive
178+
// file systems (e.g. renaming a file in a way that just changes casing), so that we support
179+
// changing the casing in the naming of the file. If this fails in any way (e.g. source file
180+
// doesn't exist, dest file doesn't exist, rename fails, etc.), we just fall back to trying the
181+
// link/unlink approach and generating any exceptional messages from there as necessary.
182+
183+
Interop.Sys.FileStatus sourceStat, destStat;
184+
if (Interop.Sys.LStat(sourceFullPath, out sourceStat) == 0 && // source file exists
185+
(Interop.Sys.LStat(destFullPath, out destStat) != 0 || // dest file does not exist
186+
(sourceStat.Dev == destStat.Dev && // source and dest are on the same device
187+
sourceStat.Ino == destStat.Ino)) && // source and dest are the same file on that device
188+
Interop.Sys.Rename(sourceFullPath, destFullPath) == 0) // try the rename
189+
{
190+
// Renamed successfully.
191+
return;
192+
}
193+
193194
LinkOrCopyFile(sourceFullPath, destFullPath);
194195
DeleteFile(sourceFullPath);
195196
}

0 commit comments

Comments
 (0)