[stdlib] Set, Dictionary: Replace _Variant enums with _BridgeStorage #19688

lorentey · 2018-10-03T16:56:44Z

This is a followup to #19611, taking it to its logical conclusion.

lorentey · 2018-10-03T16:56:53Z

@swift-ci please smoke benchmark

lorentey · 2018-10-03T16:57:03Z

lorentey · 2018-10-03T17:28:08Z

/home/buildnode/jenkins/workspace/swift-PR-Linux/branch-master/swift/stdlib/public/core/DictionaryVariant.swift:38:64: error: use of undeclared type '_NSDictionary'
12:21:03     internal var object: _BridgeStorage<_RawDictionaryStorage, _NSDictionary>
12:21:03                                                                ^~~~~~~~~~~~~

D'oh.

lorentey · 2018-10-03T17:50:43Z

The results are a weird mix. 🤔 And what's up with Dictionary.Values.swapAt(_:, _:)?

lorentey · 2018-10-03T18:05:44Z

/Users/buildnode/jenkins/workspace/swift-PR-osx/branch-master/swift/validation-test/stdlib/Inputs/DictionaryKeyValueTypesObjC.swift:24:9: error: pattern cannot match values of type 'Dictionary<KeyTy, ValueTy>._Variant'

Yeah, the tests are poking things they shouldn't.

lorentey · 2018-10-03T18:26:18Z

@swift-ci smoke benchmark

lorentey · 2018-10-03T18:26:49Z

@swift-ci smoke test

lorentey · 2018-10-03T19:27:27Z

@swift-ci test

lorentey · 2018-10-03T19:52:34Z

Remaining slowdowns affect small dictionaries/sets only. There may be a small inefficiency somewhere.

lorentey · 2018-10-03T20:09:39Z

@swift-ci please smoke test linux platform

lorentey · 2018-10-03T21:54:06Z

@swift-ci smoke test

lorentey · 2018-10-04T15:01:23Z

@swift-ci smoke test

lorentey · 2018-10-04T15:02:09Z

@swift-ci smoke benchmark

swift-ci · 2018-10-04T15:36:52Z

Build comment file:

Build failed before running benchmark.

lorentey · 2018-10-04T15:37:02Z

/Users/buildnode/jenkins/workspace/swift-PR-osx-smoke-test/branch-master/swift/stdlib/public/core/Dictionary.swift:1860:16: error: invalid redeclaration of '_isNative'
10:22:55   internal var _isNative: Bool {
10:22:55                ^
10:22:55 /Users/buildnode/jenkins/workspace/swift-PR-osx-smoke-test/branch-master/swift/stdlib/public/core/Dictionary.swift:1848:16: note: '_isNative' previously declared here
10:22:55   internal var _isNative: Bool {
10:22:55                ^

Uh, what?

lorentey · 2018-10-04T15:39:41Z

Oh, it's a merge failure.

lorentey · 2018-10-04T15:52:24Z

@swift-ci smoke test

lorentey · 2018-10-04T15:53:00Z

@swift-ci smoke benchmark

swift-ci · 2018-10-04T16:23:15Z

Build comment file:

Performance: -O

TEST	OLD	NEW	DELTA	RATIO
Regression
AnyHashableWithAClass	89161	124959	+40.1%	0.71x
DictionaryBridgeToObjC_Bridge	19	21	+10.5%	0.90x (?)
LazilyFilteredArrayContains	33415	35996	+7.7%	0.93x
Improvement
DictionaryCopy	75634	51836	-31.5%	1.46x
SetUnionInt25	153	106	-30.7%	1.44x
DictionarySubscriptDefaultMutation	347	246	-29.1%	1.41x
DictionaryFilter	61303	44603	-27.2%	1.37x
DictionaryRemove	4109	3141	-23.6%	1.31x
DictionarySubscriptDefaultMutationArray	744	576	-22.6%	1.29x
SetUnionInt50	126	99	-21.4%	1.27x
SetSymmetricDifferenceInt25	254	200	-21.3%	1.27x
CountAlgoString	3287	2615	-20.4%	1.26x
SetUnionInt0	312	254	-18.6%	1.23x
DictionarySwap	1088	896	-17.6%	1.21x
SetUnion	3136	2637	-15.9%	1.19x
SetExclusiveOr	3834	3232	-15.7%	1.19x
SetIntersectionInt100	387	327	-15.5%	1.18x
SetSymmetricDifferenceInt0	382	323	-15.4%	1.18x
SetSymmetricDifferenceInt50	267	226	-15.4%	1.18x
Histogram	621	533	-14.2%	1.17x
SetIntersectionInt50	234	202	-13.7%	1.16x
SetSymmetricDifferenceInt100	262	230	-12.2%	1.14x
Dictionary3	183	161	-12.0%	1.14x
Dictionary	408	362	-11.3%	1.13x
SetSubtractingInt100	192	172	-10.4%	1.12x
StringRemoveDupes	431	391	-9.3%	1.10x
Dictionary4OfObjects	348	316	-9.2%	1.10x
Dictionary4	280	255	-8.9%	1.10x
RGBHistogram	2307	2106	-8.7%	1.10x
SetSubtractingInt50	157	144	-8.3%	1.09x
DictionarySubscriptDefaultMutationOfObjects	1645	1517	-7.8%	1.08x

Code size: -O

TEST	OLD	NEW	DELTA	RATIO
Regression
DictTest4.o	24228	26020	+7.4%	0.93x
DictTest4Legacy.o	25218	26946	+6.9%	0.94x
DictTest3.o	27032	28520	+5.5%	0.95x
TestsUtils.o	23941	25125	+4.9%	0.95x
DictTest2.o	18824	19720	+4.8%	0.95x
DictTest.o	50391	51175	+1.6%	0.98x
Improvement
ReduceInto.o	29399	27975	-4.8%	1.05x
NSDictionaryCastToSwift.o	2065	1984	-3.9%	1.04x
DictionaryLiteral.o	1392	1344	-3.4%	1.04x
DictionaryBridgeToObjC.o	7199	6959	-3.3%	1.03x
DictionarySubscriptDefault.o	31331	30307	-3.3%	1.03x
HashQuadratic.o	5992	5800	-3.2%	1.03x
DictionarySwap.o	28342	27590	-2.7%	1.03x
DictionaryBridge.o	3715	3634	-2.2%	1.02x
DictionaryCompactMapValues.o	23002	22510	-2.1%	1.02x
DictionaryKeysContains.o	16515	16195	-1.9%	1.02x
DictionaryRemove.o	15510	15214	-1.9%	1.02x
CharacterProperties.o	18841	18489	-1.9%	1.02x
ReversedCollections.o	11171	10979	-1.7%	1.02x
SetTests.o	60745	59705	-1.7%	1.02x
StringEdits.o	14525	14300	-1.5%	1.02x
Histogram.o	4252	4188	-1.5%	1.02x
Hash.o	29683	29251	-1.5%	1.01x
CountAlgo.o	21124	20821	-1.4%	1.01x
RGBHistogram.o	25317	24997	-1.3%	1.01x

Performance: -Osize

TEST	OLD	NEW	DELTA	RATIO
Regression
AnyHashableWithAClass	110261	132541	+20.2%	0.83x
WordCountUniqueASCII	1573	1871	+18.9%	0.84x
SetIsSubsetInt50	154	177	+14.9%	0.87x
SetIntersect	614	699	+13.8%	0.88x
SetIntersectionInt0	61	69	+13.1%	0.88x
DictionaryBridgeToObjC_Bridge	19	21	+10.5%	0.90x (?)
Improvement
DictionaryCopy	77097	54719	-29.0%	1.41x
DictionaryFilter	63928	45919	-28.2%	1.39x
DictionaryRemove	4443	3424	-22.9%	1.30x
DictionarySubscriptDefaultMutation	359	286	-20.3%	1.26x
DictionarySubscriptDefaultMutationArray	761	607	-20.2%	1.25x
SetSubtractingInt100	201	162	-19.4%	1.24x
SetUnionInt25	140	113	-19.3%	1.24x
SetSubtractingInt50	161	130	-19.3%	1.24x
CountAlgoString	3106	2626	-15.5%	1.18x
Histogram	626	533	-14.9%	1.17x
SetUnionInt50	115	98	-14.8%	1.17x
SetUnionInt0	303	265	-12.5%	1.14x
SetSymmetricDifferenceInt25	237	208	-12.2%	1.14x
SetUnion	3032	2661	-12.2%	1.14x
SetSymmetricDifferenceInt50	250	221	-11.6%	1.13x
SetExclusiveOr	3777	3345	-11.4%	1.13x
Dictionary3	184	163	-11.4%	1.13x
Dictionary3OfObjects	773	686	-11.3%	1.13x
SetSymmetricDifferenceInt0	378	336	-11.1%	1.12x
Dictionary	420	375	-10.7%	1.12x
SetSymmetricDifferenceInt100	249	223	-10.4%	1.12x
StringRemoveDupes	439	396	-9.8%	1.11x
FloatingPointPrinting_Float_description_uniform	5594	5148	-8.0%	1.09x
TwoSum	1259	1165	-7.5%	1.08x
DictionarySwapAt	1109	1028	-7.3%	1.08x

Code size: -Osize

TEST	OLD	NEW	DELTA	RATIO
Regression
main.o	41785	43561	+4.3%	0.96x
DictTest4.o	20875	21275	+1.9%	0.98x
DictTest2.o	15730	16018	+1.8%	0.98x
DictTest4Legacy.o	22233	22569	+1.5%	0.99x
WordCount.o	54376	55160	+1.4%	0.99x
Improvement
ReduceInto.o	18283	17083	-6.6%	1.07x
NSError.o	1763	1683	-4.5%	1.05x
SevenBoom.o	2111	2019	-4.4%	1.05x
DictionarySubscriptDefault.o	28323	27227	-3.9%	1.04x
NSDictionaryCastToSwift.o	1941	1877	-3.3%	1.03x
HashQuadratic.o	5528	5352	-3.2%	1.03x
Histogram.o	4176	4048	-3.1%	1.03x
DictionaryCompactMapValues.o	21634	21022	-2.8%	1.03x
DictionaryBridgeToObjC.o	6749	6605	-2.1%	1.02x
DictionaryBridge.o	3751	3671	-2.1%	1.02x
DictionaryCopy.o	8313	8137	-2.1%	1.02x
DictionaryLiteral.o	1525	1493	-2.1%	1.02x
RGBHistogram.o	23381	22933	-1.9%	1.02x
DriverUtils.o	145753	143385	-1.6%	1.02x
Hash.o	22623	22271	-1.6%	1.02x
ReversedCollections.o	11597	11421	-1.5%	1.02x
DictionaryKeysContains.o	15163	14971	-1.3%	1.01x
DictionarySwap.o	25971	25659	-1.2%	1.01x
CharacterProperties.o	18842	18634	-1.1%	1.01x
DictionaryGroup.o	16167	15991	-1.1%	1.01x
SetTests.o	53273	52705	-1.1%	1.01x

Performance: -Onone

TEST	OLD	NEW	DELTA	RATIO
Regression
AnyHashableWithAClass	106177	126639	+19.3%	0.84x
Improvement
SetIsSubsetInt100	1021	929	-9.0%	1.10x
SortStringsUnicode	2892	2651	-8.3%	1.09x (?)

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

lorentey · 2018-10-04T19:47:11Z

This is looking good! I need to look at AnyHashableWithAClass and confess my ABI changes, but other than those two issues this seems ready to land.

The optimizer dislikes nested switch statements; flatten them out to simplify optimization and to hopefully speed things up a little.

…an enum

Dictionary already does this.

This is mostly a code size improvement.

…pacity The compiled code included unnecessary references to resize().

This is a tiny code size improvement.

…geObject storage We really need to do a pass on the _BridgeStorage API at some point soon.

lorentey · 2018-10-05T11:37:03Z

AnyHashableWithAClass doesn't touch sets or dictionaries. I don't see how it could be regressed by these changes, so I'm going to ignore it for now.

lorentey · 2018-10-05T13:09:51Z

@swift-ci test

swift-ci · 2018-10-05T14:22:06Z

Build failed
Swift Test Linux Platform
Git Sha - c47b922

swift-ci · 2018-10-05T14:43:22Z

Build failed
Swift Test OS X Platform
Git Sha - c47b922

lorentey · 2018-10-05T14:48:55Z

Oh yeah, this also requires a small update to LLDB's data formatters.

lorentey · 2018-10-05T20:05:47Z

Please test with the following PR:
apple/swift-lldb#959
@swift-ci please test linux platform

lorentey · 2018-10-06T00:49:04Z

Please test with the following PR:
apple/swift-lldb#959
@swift-ci please test

swift-ci · 2018-10-06T01:26:48Z

Build failed
Swift Test Linux Platform
Git Sha - aba15a1

lorentey · 2018-10-06T02:16:10Z

Hm, I don't think that Linux type checker crash is my fault.

swift-ci · 2018-10-06T03:11:51Z

Build failed
Swift Test OS X Platform
Git Sha - aba15a1

lorentey · 2018-10-07T20:25:24Z

apple/swift-lldb#959
@swift-ci please test

lorentey mentioned this pull request Oct 3, 2018

[WIP][stdlib] Finalize Hasher ABI #19685

Closed

This comment has been minimized.

Sign in to view

lorentey force-pushed the hashed-bridgeobject branch from 563fbb5 to 853720c Compare October 4, 2018 15:51

lorentey added 2 commits October 5, 2018 12:33

[stdlib] Set, Dictionary: Flatten switch statements

e24f2ba

The optimizer dislikes nested switch statements; flatten them out to simplify optimization and to hopefully speed things up a little.

[stdlib] Set, Dictionary: Switch to using a _BridgeObject instead of …

64d19f4

…an enum

lorentey added 12 commits October 5, 2018 12:33

[stdlib] Dictionary.Values.swapAt: Fix uniqueness check

12cad74

[stdlib] Rework Dictionary.values._modify

73771b1

[test] Update Set/Dictionary tests

e112de3

[test] Set and Dictionary now has an extra inhabitant

3665294

[stdlib] Set._Variant: Add a dummy initializer for use in _modify

5c0d270

Dictionary already does this.

[stdlib] Set, Dictionary: Reorganize equality checks to fix Linux builds

697ad69

[stdlib] Fix build on 32-bit platforms

21da7da

[stdlib] _Bitset: Reduce unnecessary overflow checking

be0abbb

This is mostly a code size improvement.

[stdlib] _SetBuilder, _DictionaryBuilder: Assume target has enough ca…

84d8ced

…pacity The compiled code included unnecessary references to resize().

[stdlib] Eliminate more overflow checks

bb2cb06

This is a tiny code size improvement.

[stdlib] Acknowledge there are no spare bits in Set/Dictionary’s Brid…

35502ef

…geObject storage We really need to do a pass on the _BridgeStorage API at some point soon.

[stdlib] Dictionary.Index: Remove duplicate definition for _isNative

d904b46

[test] Confess Set/Dictionary ABI changes

c47b922

lorentey force-pushed the hashed-bridgeobject branch from 853720c to c47b922 Compare October 5, 2018 13:07

lorentey mentioned this pull request Oct 5, 2018

[Swift] Add support for BridgeObject-based Set/Dictionary apple/swift-lldb#959

Merged

[test] Set and Dictionary now has an extra inhabitant (32-bit)

aba15a1

lorentey merged commit 4967393 into swiftlang:master Oct 8, 2018

lorentey deleted the hashed-bridgeobject branch October 8, 2018 11:07

lorentey mentioned this pull request Nov 28, 2018

[stdlib] Set, Dictionary: Flatten switch statements #19611

Closed

[stdlib] Set, Dictionary: Replace _Variant enums with _BridgeStorage #19688

[stdlib] Set, Dictionary: Replace _Variant enums with _BridgeStorage #19688

Uh oh!

Conversation

lorentey commented Oct 3, 2018

Uh oh!

lorentey commented Oct 3, 2018

Uh oh!

lorentey commented Oct 3, 2018

Uh oh!

This comment has been minimized.

lorentey commented Oct 3, 2018

Uh oh!

This comment has been minimized.

lorentey commented Oct 3, 2018

Uh oh!

This comment has been minimized.

lorentey commented Oct 3, 2018

Uh oh!

lorentey commented Oct 3, 2018

Uh oh!

lorentey commented Oct 3, 2018

Uh oh!

This comment has been minimized.

lorentey commented Oct 3, 2018

Uh oh!

This comment has been minimized.

lorentey commented Oct 3, 2018

Uh oh!

lorentey commented Oct 3, 2018

Uh oh!

This comment has been minimized.

lorentey commented Oct 3, 2018

Uh oh!

lorentey commented Oct 4, 2018

Uh oh!

lorentey commented Oct 4, 2018

Uh oh!

swift-ci commented Oct 4, 2018

Build comment file:

Uh oh!

lorentey commented Oct 4, 2018

Uh oh!

lorentey commented Oct 4, 2018

Uh oh!

lorentey commented Oct 4, 2018

Uh oh!

lorentey commented Oct 4, 2018

Uh oh!

swift-ci commented Oct 4, 2018

Build comment file:

Performance: -O

Code size: -O

Performance: -Osize

Code size: -Osize

Performance: -Onone

Uh oh!

lorentey commented Oct 4, 2018

Uh oh!

lorentey commented Oct 5, 2018

Uh oh!

lorentey commented Oct 5, 2018

Uh oh!

swift-ci commented Oct 5, 2018

Uh oh!

swift-ci commented Oct 5, 2018

Uh oh!

lorentey commented Oct 5, 2018

Uh oh!

lorentey commented Oct 5, 2018

Uh oh!

lorentey commented Oct 6, 2018

Uh oh!

swift-ci commented Oct 6, 2018

Uh oh!

lorentey commented Oct 6, 2018

Uh oh!

swift-ci commented Oct 6, 2018

Uh oh!

lorentey commented Oct 7, 2018