Skip to content

Conversation

@Lukasa
Copy link
Contributor

@Lukasa Lukasa commented Sep 6, 2023

While we should investigate why these regressed, for now let's unblock development.

@Lukasa Lukasa added the semver/none No version bump required. label Sep 6, 2023
@Lukasa Lukasa enabled auto-merge (squash) September 6, 2023 12:48
@Lukasa Lukasa merged commit 946d452 into apple:main Sep 6, 2023
@Lukasa Lukasa deleted the cb-alloc-limits-06-sep branch September 6, 2023 12:53
simonjbeaumont added a commit that referenced this pull request Nov 16, 2023
…tring].joined` (#161)

### Motivation

The specialization for `[Substring].joined` was dropped in Swift 5.9[^1] and consequently we regressed in allocations[^2].

### Modifications

Workaround the lack of specialized `[Substring].joined` by handwriting the comma-separated concatenation of the SSH algorithms.

### Result

Reduces the allocations to below what they were prior to the toolchain regression.

### Notes

As a baseline, I was using `d23c142` of this repo, which is a commit from around the time of the toolchain regression.

The allocation results are as follows:

| swift-nio-ssh     | swift toolchain  | allocs    |
|------------------:|:-----------------|----------:|
| d23c142 (Aug 11)  | 5.9-2023-07-25   | 1,007,000 |
|                   | 5.9-2023-07-29   | 1,100,000 |
|                   | 5.9-RELEASE       | 1,100,000 |
| d23c142 + patch   | 5.9-2023-07-25   |  989,000  |
|                   | 5.9-2023-07-29   |  989,000  |
|                   | 5.9-RELEASE       |  989,000  |
| d33c701 (main)    | 5.9-RELEASE       | 1,060,000 |
| main + patch      | 5.9-RELEASE       |  949,000  |

From this we infer:
1. The allocation regression was caused by a regression in Swift 5.9 between nightly versions `2023-07-25` and `2023-07-29`.
2. The same regression is still present in `5.9-RELEASE`.
3. The patch seems to mitigate the regression we're seeing as a result of the new 5.9 behavior.
4. Since the original regression we actually saved some 40k allocations by some other change in the code (either here, or more likely in a dependency).

For more information on the Swift issue, including reproducer package and `heaptrack` outputs, see the Swift issue[^1].

[^1]: swiftlang/swift#69883
[^2]: #157
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver/none No version bump required.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants