Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
90c02f3
Add parallel removal of items
mimischi Nov 29, 2024
ac5fe9f
Format files
mimischi Nov 30, 2024
4db67c0
Simplify structure by relying on dfs
mimischi Nov 30, 2024
09d11a9
Add documentation to new code and rename functions
mimischi Nov 30, 2024
861c188
Attempt not to break API
mimischi Nov 30, 2024
1d60824
Fix documentation check, and some indents
mimischi Dec 1, 2024
a97776b
Refactor for a cleaner structure
mimischi Dec 2, 2024
1a2f27e
Add `TokenBucket` adaptation from SwiftPM
mimischi Dec 3, 2024
2a1d510
Add `maxDescriptors` to `RemovalStrategy.swift`
mimischi Dec 3, 2024
5d7a178
Update tests
mimischi Dec 3, 2024
7a91ae6
Update `ParallelRemoval` to use `TokenBucket`
mimischi Dec 3, 2024
1b010e5
Add `FileSystemProtocol` method to not break API
mimischi Dec 3, 2024
b336a44
Actually stop breaking the API
mimischi Dec 4, 2024
0a83116
Format file
mimischi Dec 4, 2024
4390a8e
Refactor IO strategies to share commonalities
mimischi Dec 4, 2024
f00e17a
Fix typo in comment on function signature
mimischi Dec 4, 2024
1e4d5b5
Address reviews for `TokenBucket.swift`
mimischi Dec 6, 2024
dbd3ded
Address `ParallelRemoval.swift` review comments
mimischi Dec 6, 2024
5b536ca
Address `IOStrategy.swift` reviews
mimischi Dec 6, 2024
3d990b1
Remove unnecessary implementations
mimischi Dec 6, 2024
9146b91
Remove new arguments to tests function calls
mimischi Dec 6, 2024
338ff35
Re-use directory discovery logic
mimischi Dec 9, 2024
7ed6d7d
Makr `TokenBucket` as `@unchecked Sendable`
mimischi Dec 9, 2024
e9df08e
Revert "Re-use directory discovery logic"
mimischi Dec 10, 2024
e98388f
Merge branch 'main' into parallel-removal
glbrntt Dec 10, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 17 additions & 9 deletions Sources/NIOFileSystem/FileSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -438,29 +438,37 @@ public struct FileSystem: Sendable, FileSystemProtocol {
}
}

@discardableResult
private func removeItemSequentially(
at path: FilePath
) async throws -> Int {
var (subdirectories, filesRemoved) = try await self.withDirectoryHandle(
func _collectItemsInDirectory(at path: FilePath) async throws -> ([FilePath], [FilePath]) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add labels to the tuples? It's too easy to mix them up otherwise.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, do we even need to split them? We could just return the directory entries and the caller can then iterate over them and do as they need. This would avoid allocating an array unnecessarily as well (not that I think it'll have any meaningful impact on perf).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can get behind adding labels. If we only return a single array, then the callers would need to perform the sorting themselves, and thus replicate the bulk of what this utility does. I'm not wed to this existing, but I think if we have it then might as well use it to remove some level of duplication?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had a quick discussion offline. Reverted the addition of _collectItemsInDirectory in e9df08e

try await self.withDirectoryHandle(
atPath: path
) { directory in
var subdirectories = [FilePath]()
var filesRemoved = 0
var nonDirectoryItems = [FilePath]()

for try await batch in directory.listContents().batched() {
for entry in batch {
switch entry.type {
case .directory:
subdirectories.append(entry.path)

default:
filesRemoved += try await self.removeOneItem(at: entry.path)
nonDirectoryItems.append(entry.path)
}
}
}

return (subdirectories, filesRemoved)
return (subdirectories, nonDirectoryItems)
}
}

@discardableResult
private func removeItemSequentially(
at path: FilePath
) async throws -> Int {
var filesRemoved = 0
let (subdirectories, itemsToRemove) = try await self._collectItemsInDirectory(at: path)

for item in itemsToRemove {
filesRemoved += try await self.removeOneItem(at: item)
}

for subdirectory in subdirectories {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import NIOConcurrencyHelpers
/// Instead of using an ``actor``, we define a class and limit access through
/// ``NIOLock``.
@available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *)
final class TokenBucket {
final class TokenBucket: @unchecked Sendable {
private var tokens: Int
private var waiters: Deque<CheckedContinuation<Void, Never>>
private let lock: NIOLock
Expand Down
18 changes: 1 addition & 17 deletions Sources/NIOFileSystem/Internal/ParallelRemoval.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,23 +26,7 @@ extension FileSystem {
// Discover current directory and find all files/directories. Free up
// the handle as fast as possible.
let (directoriesToRecurseInto, itemsToDelete) = try await bucket.withToken {
try await self.withDirectoryHandle(atPath: path) { directory in
var subdirectories: [FilePath] = []
var itemsInDirectory: [FilePath] = []

for try await batch in directory.listContents().batched() {
for entry in batch {
switch entry.type {
case .directory:
subdirectories.append(entry.path)
default:
itemsInDirectory.append(entry.path)
}
}
}

return (subdirectories, itemsInDirectory)
}
try await self._collectItemsInDirectory(at: path)
}

return try await withThrowingTaskGroup(of: Int.self) { group in
Expand Down
Loading