realm
Use `concurrentFlatMap` in SwiftLintFramework
This has breaking changes to the SwiftLintFramework public API if anyone actually happens to be using that.
Seems like the Swift Concurrency runtime doesn't play well with tsan at all: https://forums.swift.org/t/tsan-with-task-concurrency-warnings/57775
Someone says that's fixed in Xcode 14 / Swift 5.7.
Someone says that's fixed in Xcode 14 / Swift 5.7.
Confirmed SwiftLint tests pass with tsan enabled with Xcode 14 beta 5.
| 12 Messages | |
|---|---|
| :book: | Linting Aerial with this PR took 1.16s vs 1.14s on master (1% slower) |
| :book: | Linting Alamofire with this PR took 1.38s vs 1.38s on master (0% slower) |
| :book: | Linting Firefox with this PR took 6.0s vs 6.0s on master (0% slower) |
| :book: | Linting Kickstarter with this PR took 33.09s vs 33.24s on master (0% faster) |
| :book: | Linting Moya with this PR took 0.67s vs 0.67s on master (0% slower) |
| :book: | Linting Nimble with this PR took 0.59s vs 0.6s on master (1% faster) |
| :book: | Linting Quick with this PR took 0.29s vs 0.29s on master (0% slower) |
| :book: | Linting Realm with this PR took 12.65s vs 12.65s on master (0% slower) |
| :book: | Linting SourceKitten with this PR took 0.49s vs 0.48s on master (2% slower) |
| :book: | Linting Sourcery with this PR took 2.2s vs 2.2s on master (0% slower) |
| :book: | Linting Swift with this PR took 4.16s vs 4.18s on master (0% faster) |
| :book: | Linting WordPress with this PR took 10.09s vs 10.1s on master (0% faster) |
Generated by :no_entry_sign: Danger
@SimplyDanny what do you think?
On one hand, it seems like the Swift Concurrency scheduler is more likely to see improvements than GCD concurrent perform, it seems like this is pretty close perf-wise to the previous approach, but on the other hand this makes async keywords spread through the API for no significant gain right now.
@SimplyDanny what do you think?
Without performance gains, the cluttered function interfaces and API changes I would actually postpone the update until the benefit is more visible.
Given that you have made all the work to update the interfaces already, it's also fine for me to merge it right now, however.
Yeah I'm not sold on this yet. Will hold off on merging until there's a compelling reason to do so.