[stdlib] Dictionary.values: Replace setter with _modify

[lorentey]

This should enable in-place modification of the Values view, which should greatly improve the performance of code like this:

dictionary.values[index] = newValue

https://bugs.swift.org/browse/SR-5258 / rdar://problem/44725826

[lorentey]

@swift-ci please smoke test

[lorentey]

@swift-ci please smoke benchmark

[lorentey]

I think we actually have some benchmarks for this.

[swift-ci]

Build comment file:

Build failed before running benchmark.

---

[lorentey]

Oops, Dictionary.Values.init(_variant:) has not yet landed with #19496

[lorentey]

@swift-ci please smoke benchmark

[lorentey]

@swift-ci please smoke test

[airspeedswift]

Interesting. This is similar to yielding a Slice, but sidesteps almost all of the issues slice has because Values doesn't have a public init.

Should there be a (debug?) postcondition that the user didn't replace one instance of Values with another from a different Dictionary?

[lorentey]

I guess it is slightly weird that assigning to values might affect keys, but would it cause any issues in practice?

dict.values = someOtherDict.values   // same as dict = someOtherDict

It seems technically difficult to verify the identity doesn't change. We could remember the raw bit pattern for it, but it wouldn't be foolproof -- the yieldee could end up releasing it and allocating a new one at the same address.

[airspeedswift]

Oh, right, replacing the values replaces the entire dictionary, so it is safe, if confusing. A debug trap might be useful for a case where someone mistakenly thinks it does something different though.

[swift-ci]

Build comment file:

Performance: -O

TEST	OLD	NEW	DELTA	RATIO
Improvement
DictionarySwapAt	5076	908	-82.1%	5.59x
DictionarySwapAtOfObjects	49768	9443	-81.0%	5.27x

Performance: -Osize

TEST	OLD	NEW	DELTA	RATIO
Improvement
DictionarySwapAtOfObjects	49987	10099	-79.8%	4.95x
DictionarySwapAt	5253	1328	-74.7%	3.96x

Performance: -Onone

TEST	OLD	NEW	DELTA	RATIO
Improvement
DictionarySwapAtOfObjects	113144	35401	-68.7%	3.20x
DictionarySwapAt	34148	17177	-49.7%	1.99x

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]

The setter didn't include a trap, so adding it in _modify would technically be a source-breaking change. We should probably make a separate issue for it.

[airspeedswift]

Yeah it's not a new issue for this change.