Skip to content

Refactor Mirror to reduce metadata allocation #32041

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 28, 2020

Conversation

airspeedswift
Copy link
Member

Mirror uses AnyCollection to type erase the children between different custom mirror implementations and it's own default implementation. It was previously creating an AnyCollection to store the children on creation, and this was quite costly both in object and metadata creation.

This switches to an internal Either type to store the default children implementation, and only resorts to AnyCollection for custom implementations. This avoids the box allocation if you don't access the children. Unfortunately Mirror.children: AnyCollection<Any> is ABI, but internally the standard library can take a faster path.

Refactor Mirror.descendents

Add EitherSequence

Create custom reflected children type

Switch Mirror to use EitherSequence
@airspeedswift
Copy link
Member Author

@swift-ci please benchmark

@airspeedswift airspeedswift requested review from mikeash and tbkka May 27, 2020 15:34
@airspeedswift
Copy link
Member Author

@swift-ci please test

@swift-ci
Copy link
Contributor

Performance: -O

Regression OLD NEW DELTA RATIO
Set.isSubset.Seq.Empty.Int 76 84 +10.5% 0.90x (?)
PrefixArrayLazy 13 14 +7.7% 0.93x (?)
 
Improvement OLD NEW DELTA RATIO
EqualSubstringSubstring 30 22 -26.7% 1.36x
LessSubstringSubstring 30 22 -26.7% 1.36x (?)
EqualSubstringSubstringGenericEquatable 30 22 -26.7% 1.36x
EqualSubstringString 30 22 -26.7% 1.36x
LessSubstringSubstringGenericComparable 30 22 -26.7% 1.36x
EqualStringSubstring 30 23 -23.3% 1.30x (?)
UTF8Decode_InitFromCustom_contiguous 165 144 -12.7% 1.15x (?)
StringUTF16Builder 240 210 -12.5% 1.14x
UTF8Decode_InitDecoding 163 143 -12.3% 1.14x (?)
StringComparison_longSharedPrefix 358 321 -10.3% 1.12x (?)
DistinctClassFieldAccesses 199 183 -8.0% 1.09x (?)
Set.isStrictSubset.Int.Empty 51 47 -7.8% 1.09x (?)
AngryPhonebook.Armenian 171 158 -7.6% 1.08x (?)

Code size: -O

Performance: -Osize

Regression OLD NEW DELTA RATIO
StringHasPrefixAscii 1170 1310 +12.0% 0.89x (?)
 
Improvement OLD NEW DELTA RATIO
EqualSubstringSubstring 30 22 -26.7% 1.36x
EqualSubstringSubstringGenericEquatable 30 22 -26.7% 1.36x
EqualSubstringString 30 22 -26.7% 1.36x
LessSubstringSubstring 30 23 -23.3% 1.30x
EqualStringSubstring 30 23 -23.3% 1.30x (?)
LessSubstringSubstringGenericComparable 30 23 -23.3% 1.30x
UTF8Decode_InitDecoding 164 143 -12.8% 1.15x (?)
UTF8Decode_InitFromCustom_contiguous 163 144 -11.7% 1.13x (?)
StringComparison_longSharedPrefix 359 324 -9.7% 1.11x (?)
StringUTF16Builder 230 210 -8.7% 1.10x (?)
AngryPhonebook.Cyrillic 183 169 -7.7% 1.08x (?)
AngryPhonebook.Armenian 171 158 -7.6% 1.08x (?)

Code size: -Osize

Performance: -Onone

Regression OLD NEW DELTA RATIO
StringWalk 2480 2920 +17.7% 0.85x (?)
 
Improvement OLD NEW DELTA RATIO
LessSubstringSubstring 35 27 -22.9% 1.30x
EqualSubstringSubstring 34 27 -20.6% 1.26x
EqualSubstringSubstringGenericEquatable 34 27 -20.6% 1.26x (?)
LessSubstringSubstringGenericComparable 34 27 -20.6% 1.26x (?)
EqualStringSubstring 35 28 -20.0% 1.25x
EqualSubstringString 35 28 -20.0% 1.25x
UTF8Decode_InitFromCustom_contiguous 175 153 -12.6% 1.14x (?)
UTF8Decode_InitDecoding 170 152 -10.6% 1.12x (?)
AngryPhonebook.Armenian 172 158 -8.1% 1.09x (?)
AngryPhonebook.Cyrillic 183 170 -7.1% 1.08x (?)

Code size: -swiftlibs

How to read the data The tables contain differences in performance which are larger than 8% and differences in code size which are larger than 1%.

If you see any unexpected regressions, you should consider fixing the
regressions before you merge the PR.

Noise: Sometimes the performance results (not code size!) contain false
alarms. Unexpected regressions which are marked with '(?)' are probably noise.
If you see regressions which you cannot explain you can try to run the
benchmarks again. If regressions still show up, please consult with the
performance team (@eeckstein).

Hardware Overview
  Model Name: Mac mini
  Model Identifier: Macmini8,1
  Processor Name: 6-Core Intel Core i7
  Processor Speed: 3.2 GHz
  Number of Processors: 1
  Total Number of Cores: 6
  L2 Cache (per Core): 256 KB
  L3 Cache: 12 MB
  Memory: 64 GB

@swiftlang swiftlang deleted a comment from swift-ci May 27, 2020
@airspeedswift
Copy link
Member Author

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - fc20d80

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - fc20d80

@airspeedswift
Copy link
Member Author

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - b1105ed

@airspeedswift
Copy link
Member Author

@swift-ci smoke test linux

@airspeedswift airspeedswift merged commit 5ec1e3e into swiftlang:master May 28, 2020
extension _Either {
init<T, C: Collection>(
_ collection: C
) where Right == AnyCollection<T>, C.Element == T {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can drop T by expressing this as Right == AnyCollection<C.Element>

airspeedswift added a commit that referenced this pull request Jul 17, 2020
- Refactor Mirror.descendents

- Add _Either sequence

- Create custom reflected children type

- Switch Mirror to use _Either

Co-authored-by: Ben Cohen <[email protected]>
airspeedswift added a commit that referenced this pull request Aug 6, 2020
airspeedswift added a commit that referenced this pull request Aug 6, 2020
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.

4 participants