Skip to content

Conversation

@allevato
Copy link
Member

I ran the old version and the new version in release mode for 15 iterations over about 400 source files and got the following mean wall times:

  • lint: 9.71 sec (Dispatch) vs. 9.83 sec (Concurrency)
  • format: 20.61 sec (Dispatch) vs. 20.79 (Concurrency)

Even though the means for Concurrency are slightly higher, Student's t-test yielded p = 0.416 (lint) and p = 0.439 (format) which aren't low enough to claim they're significant.

This replaces `Dispatch.concurrentPerform` with a task group. One
benefit of this approach is that we don't need to know the number
of iterations that we want ahead of time. We can start tasks during
the initial iteration over the `FileIterator` instead of eagerly
expanding it first; this should potentially improve behavior for
invocations that cover very large directories recursively,
especially when they're on slower storage like a network drive.
Instead of having `StderrDiagnosticPrinter` internally manage its
own synchronization (which currently uses a Dispatch queue), turn
`DiagnosticsEngine` into an actor that processes diagnostics from
an `AsyncStream`. When rules emit findings, they are enqueued in
the stream, and a single task iterates over them to ensure that
individual diagnostic handlers don't need to synchronize their own
activity.
@a7medev
Copy link
Member

a7medev commented Oct 23, 2025

@allevato Just curious, is there any reason why swift-format hasn't migrated to Swift concurrency yet?

@allevato
Copy link
Member Author

Mainly just that I wanted to do further testing to make sure that the new implementation didn't try to overcommit the thread pool when the number of files is significantly higher than the number of cores. That doesn't come automatically with Swift concurrency whereas Dispatch's concurrentPerform is supposed to scale properly for that—although issues like #1069 lead me to believe the current implementation has problems there too.

@MaxDesiatov
Copy link
Contributor

How hard would it be to conditionally switch to Swift Concurrency only when #if canImport(Dispatch) fails? It looks like the PR is fairly small, if its diff amounts to the only changes needed for Swift Concurrency support. If you still have reservations about migration from Dispatch on other platforms, we could at least adopt Swift Concurrency on platforms that have no Dispatch at all?

@allevato
Copy link
Member Author

We must have very different definitions of "small" 🙂

I'm very uninterested in maintaining both implementations simultaneously for any period of time. This really just needs me to carve out a little bit more time to do some further testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants