[SDK] Use withContiguousStorageIfAvailable in some overlay methods

[karwa]

No description provided.

[karwa]

CC @moiseev @milseman

Now that we have a consistent and public way to access the underlying contiguous storage of generic Sequences, we should use it. For one thing it lets you use slices and unsafe buffers without copying.

[phausler]

Looks good to me

[moiseev]

Minor stylistic nitpick. Just discussed it with @milseman and we agreed that it would be more readable if these two lines are reformatted as the following:

withContiguousStorageIfAvailable(_clip)
?? Array(self).withUnsafeBufferPointer(_clip)

This way it is clear that the second line is not a new statement, but instead is a previous expression continued.
What do you think?

[karwa]

Sounds good. Updated.

[moiseev]

Same here.

guard let str = bytes.withContiguousStorageIfAvailable(_makeString)
             ?? Array(bytes).withUnsafeBufferPointer(_makeString) else {
  return nil
}

[milseman]

That conflict is from #21959. When you resolve it, can you make sure we still form native strings from valid UTF-8 contents? The benchmarks will reflect this improvement as well.

[karwa]

@milseman Rebased.

[milseman]

@swift-ci please benchmark

[swift-ci]

Build comment file:

Performance: -O

TEST	OLD	NEW	DELTA	RATIO
Regression
UTF8Decode_InitFromBytes_ascii	301	427	+41.9%	0.70x
UTF8Decode_InitFromBytes	247	272	+10.1%	0.91x (?)

Performance: -Osize

TEST	OLD	NEW	DELTA	RATIO
Regression
UTF8Decode_InitFromBytes_ascii	302	424	+40.4%	0.71x
UTF8Decode_InitFromBytes	249	272	+9.2%	0.92x (?)
Improvement
ObjectiveCBridgeStubURLAppendPathRef2	2769	2587	-6.6%	1.07x (?)

Performance: -Onone

TEST	OLD	NEW	DELTA	RATIO
Regression
UTF8Decode_InitFromBytes_ascii	298	431	+44.6%	0.69x
UTF8Decode_InitFromBytes	247	273	+10.5%	0.90x (?)

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

--------------

[karwa]

This doesn't really show the benefit because those benchmarks use Array, which wouldn't copy even in unspecialised generic code.

Created #22277 to add benchmarks for UBP, URBP. The same should apply to Slice/ArraySlice.

[milseman]

Ok, do you know why there is a 41% regression when it is an array, though?

[karwa]

Not yet, I'll check it out. I've found these benchmarks to be pretty noisy locally.

In any case, I think we could speed it up a teeny bit more by copying to ContiguousArray.

[shahmishal]

Please update the base branch to main by Oct 5th otherwise the pull request will be closed automatically.

• How to change the base branch: (Link)
• More detail about the branch update: (Link)