Enable run-time exclusivity checking in release mode. #20302
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change could impact Swift programs that previously appeared well-behaved, but weren't fully tested in debug mode. Now, when running in release mode, they may trap with the message "error: overlapping accesses...". Recent optimizations have brought performance where I think it needs to be for adoption. More optimizations are planned, and some benchmarks should be further improved, but at this point we're ready to begin receiving bug reports. That will help prioritize the remaining work for Swift 5. Of the 656 public microbenchmarks in the Swift repository, there are still several regressions larger than 10%: TEST OLD NEW DELTA RATIO ClassArrayGetter2 139 1307 +840.3% **0.11x** HashTest 631 1233 +95.4% **0.51x** NopDeinit 21269 32389 +52.3% **0.66x** Hanoi 1478 2166 +46.5% **0.68x** Calculator 127 158 +24.4% **0.80x** Dictionary3OfObjects 391 455 +16.4% **0.86x** CSVParsingAltIndices2 526 604 +14.8% **0.87x** Prims 549 626 +14.0% **0.88x** CSVParsingAlt2 1252 1411 +12.7% **0.89x** Dictionary4OfObjects 206 232 +12.6% **0.89x** ArrayInClass 46 51 +10.9% **0.90x** The common pattern in these benchmarks is to define an array of data as a class property and to repeatedly access that array through the class reference. Each of those class property accesses now incurs a runtime call. Naturally, introducing a runtime call in a loop that otherwise does almost no work incurs substantial overhead. This is similar to the issue caused by automatic reference counting. In some cases, more sophistacated optimization will be able to determine the same object is repeatedly accessed. Furthermore, the overhead of the runtime call itself can be improved. But regardless of how well we optimize, there will always a class of microbenchmarks in which the runtime check has a noticeable impact. As a general guideline, avoid performing class property access within the most performance critical loops, particularly on different objects in each loop iteration. If that isn't possible, it may help if the visibility of those class properties is private or internal.
|
@swift-ci test. |
|
@swift-ci test source compatibility. |
|
@swift-ci benchmark. |
Build comment file:Performance: -O
Code size: -O
Performance: -Osize
Code size: -Osize
How to read the dataThe 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 |
|
Build failed |
|
@swift-ci smoke test linux. |
|
I'm seeing the same linux crash on master without these changes: It's not clear how other PR tests have passed. |
|
@swift-ci smoke test linux. |
|
@swift-ci smoke test linux platform |
|
@atrick I restarted the smoke test linux job to make sure you got that commit so you can see the error. |
|
@swift-ci clean test linux platform. |
|
Build failed |
|
@swift-ci smoke test linux platform |
|
@swift-ci smoke test Linux platform |
|
@swift-ci smoke test. |
|
Congrats @atrick! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It's time to change the compiler default for run-time exclusivity
checks. This will expose the feature to more testing and allow the
compiler team to gather performance feedback from users. Until now,
I've been waiting until the most obvious bottlenecks are
resolved--there's no sense having a slew of performance bugs for
issues that are about to be fixed. Recent optimizations landed by Joe
Shajrawi have brought performance where I think it needs to be for
adoption. More optimizations are planned, and some benchmarks should
be further improved, but at this point we're ready to begin receiving
bug reports. That will help prioritize the remaining work for Swift 5.
I'll post to swift-announce shortly.
Fixes rdar://problem/37830912 [SR-7139]: [Exclusivity] Enable -exclusivity-enforcement=checked (dynamic checks) by default in release (-O) mode.
This change could impact Swift programs that previously appeared
well-behaved, but weren't fully tested in debug mode. Now, when running
in release mode, they may trap with the message "error: overlapping
accesses...".
Recent optimizations have brought performance where I think it needs
to be for adoption. More optimizations are planned, and some
benchmarks should be further improved, but at this point we're ready
to begin receiving bug reports. That will help prioritize the
remaining work for Swift 5.
Of the 656 public microbenchmarks in the Swift repository, there are
still several regressions larger than 10%:
The common pattern in these benchmarks is to define an array of data
as a class property and to repeatedly access that array through the
class reference. Each of those class property accesses now incurs a
runtime call. Naturally, introducing a runtime call in a loop that
otherwise does almost no work incurs substantial overhead. This is
similar to the issue caused by automatic reference counting. In some
cases, more sophistacated optimization will be able to determine the
same object is repeatedly accessed. Furthermore, the overhead of the
runtime call itself can be improved. But regardless of how well we
optimize, there will always a class of microbenchmarks in which the
runtime check has a noticeable impact.
As a general guideline, avoid performing class property access within
the most performance critical loops, particularly on different objects
in each loop iteration. If that isn't possible, it may help if the
visibility of those class properties is private or internal.