Skip to content

Commit

Permalink
Clone files on Darwin rather than copying them
Browse files Browse the repository at this point in the history
Motivation:

Copying regular files on Darwin was taking more time than it needed to
because it was manually opening files and copying over bites rather than
making use of the system's ability to create CoW clones.

Modifications:

The primary change is to switch `copyItem` to be backed by Darwin's
`copyfile` method rather than `fcopyfile`. `fcopyfile` ignores the flag
we were passing in to indicate that the file should be cloned if
possible, whereas `copyfile` is a higher-level API which takes this into
account.

This change exposes a `copyfile` wrapper method as a new public
function.

This change also adds a workaround to a seeming compiler bug on channel
options blocking building tests on more recent Xcodes.

Result:

Copying files on Darwin will be faster.
  • Loading branch information
rnro committed Aug 2, 2024
1 parent ba62457 commit adeb5b6
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 31 deletions.
42 changes: 22 additions & 20 deletions Sources/NIOFileSystem/FileSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1172,6 +1172,27 @@ extension FileSystem {
)
}

#if canImport(Darwin)
// COPYFILE_CLONE clones the file if possible and will fallback to doing a copy.
// COPYFILE_ALL is shorthand for:
// COPYFILE_STAT | COPYFILE_ACL | COPYFILE_XATTR | COPYFILE_DATA
let flags = copyfile_flags_t(COPYFILE_CLONE) | copyfile_flags_t(COPYFILE_ALL)
return Libc.copyfile(
from: sourcePath,
to: destinationPath,
state: nil,
flags: flags
).mapError { errno in
FileSystemError.fcopyfile(
errno: errno,
from: sourcePath,
to: destinationPath,
location: .here()
)
}

#elseif canImport(Glibc) || canImport(Musl) || canImport(Bionic)

let openSourceResult = self._openFile(
forReadingAt: sourcePath,
options: OpenOptions.Read(followSymbolicLinks: true)
Expand Down Expand Up @@ -1231,25 +1252,6 @@ extension FileSystem {
let copyResult: Result<Void, FileSystemError>
copyResult = source.fileHandle.systemFileHandle.sendableView._withUnsafeDescriptorResult { sourceFD in
destination.fileHandle.systemFileHandle.sendableView._withUnsafeDescriptorResult { destinationFD in
#if canImport(Darwin)
// COPYFILE_CLONE clones the file if possible and will fallback to doing a copy.
// COPYFILE_ALL is shorthand for:
// COPYFILE_STAT | COPYFILE_ACL | COPYFILE_XATTR | COPYFILE_DATA
let flags = copyfile_flags_t(COPYFILE_CLONE) | copyfile_flags_t(COPYFILE_ALL)
return Libc.fcopyfile(
from: sourceFD,
to: destinationFD,
state: nil,
flags: flags
).mapError { errno in
FileSystemError.fcopyfile(
errno: errno,
from: sourcePath,
to: destinationPath,
location: .here()
)
}
#elseif canImport(Glibc) || canImport(Musl) || canImport(Bionic)
var offset = 0

while offset < sourceInfo.size {
Expand Down Expand Up @@ -1277,7 +1279,6 @@ extension FileSystem {
}
}
return .success(())
#endif
} onUnavailable: {
makeOnUnavailableError(path: destinationPath, location: .here())
}
Expand All @@ -1287,6 +1288,7 @@ extension FileSystem {

let closeResult = destination.fileHandle.systemFileHandle.sendableView._close(materialize: true)
return copyResult.flatMap { closeResult }
#endif
}

private func copySymbolicLink(
Expand Down
19 changes: 19 additions & 0 deletions Sources/NIOFileSystem/FileSystemError+Syscall.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1016,6 +1016,25 @@ extension FileSystemError {
Can't copy file from '\(sourcePath)' to '\(destinationPath)', the destination \
path already exists.
"""
case .fileExists:
code = .fileAlreadyExists
message = """
Unable to create file at path '\(destinationPath)', no existing file options were set \
which implies that no file should exist but a file already exists at the \
specified path.
"""
case .tooManyOpenFiles:
code = .unavailable
message = """
Unable to open the source ('\(sourcePath)') or destination ('\(destinationPath)') files, \
too many file descriptors are open.
"""
case .noSuchFileOrDirectory:
code = .notFound
message = """
Unable to open or create file at path '\(sourcePath)', either a component of the \
path did not exist or the named file to be opened did not exist.
"""
default:
code = .unknown
message = "Can't copy file from '\(sourcePath)' to '\(destinationPath)'."
Expand Down
18 changes: 18 additions & 0 deletions Sources/NIOFileSystem/Internal/System Calls/Syscall.swift
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,24 @@ public enum Libc {
}
#endif

#if canImport(Darwin)
@_spi(Testing)
public static func copyfile(
from source: FilePath,
to destination: FilePath,
state: copyfile_state_t?,
flags: copyfile_flags_t
) -> Result<Void, Errno> {
source.withPlatformString { sourcePath in
destination.withPlatformString { destinationPath in
nothingOrErrno(retryOnInterrupt: false) {
libc_copyfile(sourcePath, destinationPath, state, flags)
}
}
}
}
#endif

@_spi(Testing)
public static func remove(
_ path: FilePath
Expand Down
19 changes: 18 additions & 1 deletion Sources/NIOFileSystem/Internal/System Calls/Syscalls.swift
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ internal func libc_remove(
}

#if canImport(Darwin)
/// copyfile(3): Copy a file from one file to another.
/// copyfile(3): Copy a file from one file to another. (fcopyfile)
internal func libc_fcopyfile(
_ from: CInt,
_ to: CInt,
Expand All @@ -416,6 +416,23 @@ internal func libc_fcopyfile(
}
#endif

#if canImport(Darwin)
/// copyfile(3): Copy a file from one file to another. (copyfile)
internal func libc_copyfile(
_ from: UnsafePointer<CInterop.PlatformChar>,
_ to: UnsafePointer<CInterop.PlatformChar>,
_ state: copyfile_state_t?,
_ flags: copyfile_flags_t
) -> CInt {
#if ENABLE_MOCKING
if mockingEnabled {
return mock(from, to, state, flags)
}
#endif
return copyfile(from, to, state, flags)
}
#endif

/// getcwd(3): Get working directory pathname
internal func libc_getcwd(
_ buffer: UnsafeMutablePointer<CInterop.PlatformChar>,
Expand Down
14 changes: 7 additions & 7 deletions Tests/NIOFileSystemIntegrationTests/FileSystemTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -659,14 +659,14 @@ final class FileSystemTests: XCTestCase {
/// This is is not quite the same as sequential, different code paths are used.
/// Tests using this ensure use of the parallel paths (which are more complex) while keeping actual
/// parallelism to minimal levels to make debugging simpler.
private let minimalParallel: CopyStrategy = try! .parallel(maxDescriptors: 2)
private static let minimalParallel: CopyStrategy = try! .parallel(maxDescriptors: 2)

func testCopyEmptyDirectorySequential() async throws {
try await testCopyEmptyDirectory(.sequential)
}

func testCopyEmptyDirectoryParallelMinimal() async throws {
try await testCopyEmptyDirectory(minimalParallel)
try await testCopyEmptyDirectory(Self.minimalParallel)
}

func testCopyEmptyDirectoryParallelDefault() async throws {
Expand All @@ -690,7 +690,7 @@ final class FileSystemTests: XCTestCase {
}

func testCopyOnGeneratedTreeStructureParallelMinimal() async throws {
try await testAnyCopyStrategyOnGeneratedTreeStructure(minimalParallel)
try await testAnyCopyStrategyOnGeneratedTreeStructure(Self.minimalParallel)
}

func testCopyOnGeneratedTreeStructureParallelDefault() async throws {
Expand Down Expand Up @@ -737,7 +737,7 @@ final class FileSystemTests: XCTestCase {
}

func testCopySelectivelyParallelMinimal() async throws {
try await testCopySelectively(minimalParallel)
try await testCopySelectively(Self.minimalParallel)
}

func testCopySelectivelyParallelDefault() async throws {
Expand Down Expand Up @@ -780,7 +780,7 @@ final class FileSystemTests: XCTestCase {
}

func testCopyCancelledPartWayThroughParallelMinimal() async throws {
try await testCopyCancelledPartWayThrough(minimalParallel)
try await testCopyCancelledPartWayThrough(Self.minimalParallel)
}

func testCopyCancelledPartWayThroughParallelDefault() async throws {
Expand Down Expand Up @@ -927,7 +927,7 @@ final class FileSystemTests: XCTestCase {
}

func testCopyNonExistentFileParallelMinimal() async throws {
try await testCopyNonExistentFile(minimalParallel)
try await testCopyNonExistentFile(Self.minimalParallel)
}

func testCopyNonExistentFileParallelDefault() async throws {
Expand All @@ -953,7 +953,7 @@ final class FileSystemTests: XCTestCase {
}

func testCopyToExistingDestinationParallelMinimal() async throws {
try await testCopyToExistingDestination(minimalParallel)
try await testCopyToExistingDestination(Self.minimalParallel)
}

func testCopyToExistingDestinationParallelDefault() async throws {
Expand Down
5 changes: 2 additions & 3 deletions Tests/NIOPosixTests/StreamChannelsTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -365,9 +365,8 @@ class StreamChannelTest: XCTestCase {
)

// We will immediately send exactly the amount of data that fits in the receiver's receive buffer.
let receiveBufferSize = Int(
(try? receiver.getOption(.socketOption(.so_rcvbuf)).wait()) ?? 8192
)
let opt = try? receiver.getOption(.socketOption(.so_rcvbuf)).wait()
let receiveBufferSize = Int(opt ?? 8192)
var buffer = sender.allocator.buffer(capacity: receiveBufferSize)
buffer.writeBytes(Array(repeating: UInt8(ascii: "X"), count: receiveBufferSize))

Expand Down

0 comments on commit adeb5b6

Please sign in to comment.