Adds KeyPath test to benchmark suite by taylorGeisler · Pull Request #36451 · swiftlang/swift

taylorGeisler · 2021-03-16T18:46:03Z

This pull request adds a test of KeyPaths performance to the benchmark suite. It creates a fixed size array struct and KeyPaths to its properties. It does work inside of a loop by accessing the properties with KeyPaths.

CodaFi · 2021-03-20T19:08:23Z

@swift-ci smoke test

CodaFi · 2021-03-20T19:08:28Z

@swift-ci benchmark

swift-ci · 2021-03-20T19:38:18Z

Performance: -O

Regression	OLD	NEW	DELTA	RATIO
ObjectiveCBridgeStubToNSStringRef	110	127	+15.5%	0.87x (?)

Improvement	OLD	NEW	DELTA	RATIO
FlattenListFlatMap	6618	3925	-40.7%	1.69x (?)
NSError	269	162	-39.8%	1.66x (?)
NSStringConversion.UTF8	1056	980	-7.2%	1.08x (?)

Added	MIN	MAX	MEAN	MAX_RSS
KeyPath	1359	1421	1380	—

Code size: -O

Performance: -Osize

Improvement	OLD	NEW	DELTA	RATIO
StringFromLongWholeSubstring	5	4	-20.0%	1.25x
CharacterLiteralsLarge	111	100	-9.9%	1.11x (?)
UTF8Decode_InitFromData_ascii_as_ascii	697	629	-9.8%	1.11x (?)
CharacterLiteralsSmall	345	322	-6.7%	1.07x (?)

Added	MIN	MAX	MEAN	MAX_RSS
KeyPath	1354	1410	1373	—

Code size: -Osize

Performance: -Onone

Improvement	OLD	NEW	DELTA	RATIO
DataToStringLargeUnicode	9450	8600	-9.0%	1.10x (?)

Added	MIN	MAX	MEAN	MAX_RSS
KeyPath	1191092	1201186	1194621	—

Code size: -swiftlibs

✅	Benchmark Check Report
⛔️⏱	`KeyPath` has setup overhead of 1368 μs (100.1%). _{Move initialization of benchmark data to the setUpFunction registered in BenchmarkInfo.}
⚠️⏱	`KeyPath` execution took -1 μs. _{Increase the workload of KeyPath to be more than 20 μs.}

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 Pro
  Model Identifier: MacPro6,1
  Processor Name: 12-Core Intel Xeon E5
  Processor Speed: 2.7 GHz
  Number of Processors: 1
  Total Number of Cores: 12
  L2 Cache (per Core): 256 KB
  L3 Cache: 30 MB
  Memory: 64 GB

eeckstein

The key path accesses are all optimized away. You can use identity to prevent that, e.g.

    let kp0 = identity(FixedSizeArray10<Double>.getKeypathToElement(index: 0))

Also, it would be nice to benchmark other key path operations, like: subscripts, getters, setters.

Adds identity function to avoid keypath being optimized away

taylorGeisler · 2021-03-24T14:55:26Z

@swift-ci benchmark

eeckstein · 2021-03-25T07:52:51Z

@swift-ci benchmark

swift-ci · 2021-03-25T08:17:33Z

Build failed before running benchmark.

taylorGeisler · 2021-03-29T19:13:49Z

@eeckstein Overall I'd like this benchmark to highlight the performance difference between the KeyPath and Direct Access implementations in working with struct properties. Any further suggestions?

eeckstein · 2021-03-30T08:03:36Z

Overall I'd like this benchmark to highlight the performance difference between the KeyPath and Direct Access implementations

The difference between both benchmarks is not reported in a benchmark run, e.g. on CI. So the only way to see the difference is to compile the benchmarks locally and manually compare both results.
And runStructDirectAccessComputation as a stand-alone benchmark is not very useful, IMO.

You can keep runStructDirectAccessComputation if you think you'd like to do this manual comparison by your own, but then please add the skip tag, so that it does not run by default.
Or you just delete the benchmark.

eeckstein

lgtm, thanks!

eeckstein · 2021-03-31T10:00:25Z

@swift-ci benchmark

eeckstein · 2021-03-31T10:00:34Z

@swift-ci smoke test

eeckstein · 2021-03-31T10:51:08Z

@swift-ci benchmark

swift-ci · 2021-03-31T11:30:12Z

Performance: -O

Regression	OLD	NEW	DELTA	RATIO
FlattenListLoop	1631	2510	+53.9%	0.65x (?)
FlattenListFlatMap	4432	6712	+51.4%	0.66x (?)

Improvement	OLD	NEW	DELTA	RATIO
DictionaryKeysContainsNative	30	26	-13.3%	1.15x (?)
ObjectiveCBridgeStubToNSStringRef	119	110	-7.6%	1.08x (?)

Added	MIN	MAX	MEAN	MAX_RSS
StructKeyPathComputation	1216726	1219019	1217986	—

Code size: -O

Performance: -Osize

Improvement	OLD	NEW	DELTA	RATIO
ObjectiveCBridgeStubToNSStringRef	133	121	-9.0%	1.10x (?)
ObjectiveCBridgeFromNSArrayAnyObjectForced	5200	4760	-8.5%	1.09x (?)

Added	MIN	MAX	MEAN	MAX_RSS
StructKeyPathComputation	1287360	1297421	1293962	—

Code size: -Osize

Performance: -Onone

Regression	OLD	NEW	DELTA	RATIO
NSError	728	791	+8.7%	0.92x (?)

Added	MIN	MAX	MEAN	MAX_RSS
StructKeyPathComputation	1436021	1460507	1451122	—

Code size: -swiftlibs

✅	Benchmark Check Report
⛔️⏱	`StructKeyPathComputation` has setup overhead of 1315368 μs (104.6%). _{Move initialization of benchmark data to the setUpFunction registered in BenchmarkInfo.}
⚠️⏱	`StructKeyPathComputation` execution took -58365 μs. _{Increase the workload of StructKeyPathComputation to be more than 20 μs.}

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 Pro
  Model Identifier: MacPro6,1
  Processor Name: 12-Core Intel Xeon E5
  Processor Speed: 2.7 GHz
  Number of Processors: 1
  Total Number of Cores: 12
  L2 Cache (per Core): 256 KB
  L3 Cache: 30 MB
  Memory: 64 GB

eeckstein · 2021-03-31T13:08:51Z

@taylorGeisler I just saw that you moved the keypath variables out of the function and made them global. That is not good, because accesses to (non-trivial) globals is slow.
Please move those variables back into the function.

BradLarson · 2022-08-03T21:14:21Z

My coworker (from the same company that originated this PR) has been working on a more extensive set of keypath benchmarks in #60383, so I believe it is safe to close out this PR in favor of that newer one.

Adds KeyPath test to benchmark suite

15f263a

CodaFi requested a review from jckarter March 20, 2021 19:09

eeckstein requested changes Mar 22, 2021

View reviewed changes

Update KeyPath.swift

ad65193

Adds identity function to avoid keypath being optimized away

taylorGeisler added 2 commits March 29, 2021 13:04

Add comparison of KeyPath to Direct Access

215d1fc

Remove extra function

f70f388

taylorGeisler requested a review from eeckstein March 29, 2021 19:09

Remove runStructDirectAccessComputation benchmark

7baa7e0

eeckstein approved these changes Mar 31, 2021

View reviewed changes

fibrechannelscsi mentioned this pull request Aug 3, 2022

Add benchmarks that measure KeyPath read and write performance. #60383

Merged

BradLarson closed this Aug 3, 2022

Conversation

taylorGeisler commented Mar 16, 2021

Uh oh!

CodaFi commented Mar 20, 2021

Uh oh!

CodaFi commented Mar 20, 2021

Uh oh!

swift-ci commented Mar 20, 2021

Performance: -O

Code size: -O

Performance: -Osize

Code size: -Osize

Performance: -Onone

Code size: -swiftlibs

Uh oh!

eeckstein left a comment

Choose a reason for hiding this comment

Uh oh!

taylorGeisler commented Mar 24, 2021

Uh oh!

eeckstein commented Mar 25, 2021

Uh oh!

swift-ci commented Mar 25, 2021

Uh oh!

taylorGeisler commented Mar 29, 2021

Uh oh!

eeckstein commented Mar 30, 2021

Uh oh!

eeckstein left a comment

Choose a reason for hiding this comment

Uh oh!

eeckstein commented Mar 31, 2021

Uh oh!

eeckstein commented Mar 31, 2021

Uh oh!

eeckstein commented Mar 31, 2021

Uh oh!

swift-ci commented Mar 31, 2021

Performance: -O

Code size: -O

Performance: -Osize

Code size: -Osize

Performance: -Onone

Code size: -swiftlibs

Uh oh!

eeckstein commented Mar 31, 2021

Uh oh!

BradLarson commented Aug 3, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants