Skip to content

Conversation

@oxy
Copy link
Contributor

@oxy oxy commented May 23, 2023

When calling Array.removeAll(keepingCapacity: true), bypass the replaceSubrange call unless the buffer is uniquely referenced. Also, add a benchmark to measure performance for the new branch.

Based on @glessard's work in #39784.
Resolves #56321 (rdar://71809690).

@oxy oxy changed the title Gh56321 [stdlib] don't copy array contents on removeAll(keepingCapacity: true) May 23, 2023
@phausler
Copy link
Contributor

@swift-ci please test

@oxy
Copy link
Contributor Author

oxy commented May 24, 2023

@swift-ci please benchmark

@oxy
Copy link
Contributor Author

oxy commented May 24, 2023

Benchmark result summary:

  • +1-5% codesize overhead on -Osize and -O (Array.removeAll is @inlinable)
  • Array.removeAll(keepingCapacity: true) is now constant-time for arrays that are not uniquely referenced. (350-600us -> 0-5us with the current benchmark)

Details:

Performance (x86_64): -O

Improvement OLD NEW DELTA RATIO
Array.removeAll.keepingCapacity.Int 601.0 0.102 -100.0% 5834.96x
Array.removeAll.keepingCapacity.Object 358.0 5.727 -98.4% 62.50x

Code size: -O

Regression OLD NEW DELTA RATIO
ArrayRemoveAll.o 7464 7896 +5.8% 0.95x
MirrorTest.o 11422 11604 +1.6% 0.98x

Code size: -Osize

Regression OLD NEW DELTA RATIO
ArrayRemoveAll.o 6969 7374 +5.8% 0.95x
StringSplitting.o 34491 36318 +5.3% 0.95x
MirrorTest.o 11298 11468 +1.5% 0.99x

@oxy oxy requested a review from lorentey May 24, 2023 16:40
@oxy
Copy link
Contributor Author

oxy commented May 24, 2023

@swift-ci please test

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.

[SR-13923] Array.removeAll(keepingCapacity: true) wastes time copying memory around when Array is not uniquely referenced

3 participants