attributed string fast path equality performance improvements

[vanvoorden]

Here is a small performance improvement to value-equality checking in AttributedString.

When two AttributedString instances are identical we can return true in constant-time: we do not need to check through all N characters.

Currently the check for identity equality lives inside Guts.¹²³ This is still constant-time… but we have the potential to move this check up sooner: directly in the == operator on AttributedString. This will look similar to what we already have in place for AttributedSubstring.⁴

Benchmarks show improvements close to two orders of magnitude increased throughput when our AttributedString instances are identical.

SWIFTCI_USE_LOCAL_DEPS=1 swift package --package-path Benchmarks --disable-sandbox -c release benchmark run --target "AttributedStringBenchmarks" --filter "equality" --format markdown

SWIFTCI_USE_LOCAL_DEPS=1 swift package --package-path Benchmarks --disable-sandbox -c release benchmark run --target "AttributedStringBenchmarks" --filter "equalityShared" --format markdown

Control: Equality Unique

Metric	p0	p25	p50	p75	p90	p99	p100	Samples
Throughput (# / s) (K)	81	79	78	78	77	77	77	79
Time (total CPU) (μs) *	12	13	13	13	13	13	13	79
Time (wall clock) (μs) *	12	13	13	13	13	13	13	79

Test: Equality Unique

Metric	p0	p25	p50	p75	p90	p99	p100	Samples
Throughput (# / s) (K)	84	80	79	79	78	77	77	80
Time (total CPU) (μs) *	12	13	13	13	13	13	13	80
Time (wall clock) (μs) *	12	13	13	13	13	13	13	80

Control: Equality Shared

Metric	p0	p25	p50	p75	p90	p99	p100	Samples
Throughput (# / s) (M)	421	381	381	375	369	296	12	10000
Time (total CPU) (ns) *	3	3	3	3	3	4	83	10000
Time (wall clock) (ns) *	2	3	3	3	3	3	81	10000

Test: Equality Shared

Metric	p0	p25	p50	p75	p90	p99	p100	Samples
Throughput (# / s) (G)	24	24	24	24	24	12	0	9634
Time (total CPU) (ns) *	1	1	1	1	1	1	14	10000
Time (wall clock) (ns) *	0	0	0	0	0	0	5	9634

Footnotes

1. https://github.com/swiftlang/swift-foundation/blob/swift-6.1.2-RELEASE/Sources/FoundationEssentials/AttributedString/AttributedString%2BGuts.swift#L77 ↩

2. https://github.com/swiftlang/swift-foundation/blob/swift-6.1.2-RELEASE/Sources/FoundationEssentials/AttributedString/AttributedString%2BGuts.swift#L84-L86 ↩

3. https://github.com/swiftlang/swift-foundation/blob/swift-6.1.2-RELEASE/Sources/FoundationEssentials/AttributedString/AttributedString%2BGuts.swift#L105 ↩

4. https://github.com/swiftlang/swift-foundation/blob/swift-6.1.2-RELEASE/Sources/FoundationEssentials/AttributedString/AttributedSubstring.swift#L79-L81 ↩

[jmschonfeld]

@swift-ci please test

[jmschonfeld]

Thanks for updating this and for including the benchmark results! Indeed this change seems sensible especially since AttributedSubstring already does this. I'm curious to know exactly what work we're eliding here - I suspect it's the creation and comparison of a RangeSet representing the full range of the string when we create the AttributedString.Runs instances to compare. Out of curiosity, did you profile the call before/after to see what calls were previously taking up the most time in the identical case?

[vanvoorden]

Out of curiosity, did you profile the call before/after to see what calls were previously taking up the most time in the identical case?

I did not do much profiling outside of running benchmarks. But skipping all the work in the AttributedString.Runs constructor looks like where we are seeing the wins from.

[vanvoorden]

@jmschonfeld I think we fixed the Windows issue in #1416. Do I have to merge main here in this diff or will running the job again pick up the new changes?

[jmschonfeld]

@jmschonfeld I think we fixed the Windows issue in #1416. Do I have to merge main here in this diff or will running the job again pick up the new changes?

I believe CI checks out the merge commit rather than your branch, so I think re-running should be sufficient - let me try re-running it and if it fails again we can rebase/merge main in

[jmschonfeld]

@swift-ci please test Windows platform

[vanvoorden]

@jmschonfeld I think this looks good to merge. Thanks!

[jmschonfeld]

Thanks! Yep this looks good to me - I'll get this merged soon once we finish sorting out a few other action items for the 6.2 release