Skip to content

[opt] promote stack alloc_ref to object #30743

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

zoecarver
Copy link
Contributor

Based on #30736. This patch updates StackPromotion to promote alloc_refs on the stack to object instructions. Given that there's currently no support for object instructions in the optimizer, this pass shouldn't have many (if any) performance improvements. It just lays the groundwork for future performance improvements.

In the future, the promotion could probably be more aggressive but, as of now, it will only promote stack alloc_refs so there shouldn't be any regression.

@zoecarver
Copy link
Contributor Author

@swift-ci please benchmark

@zoecarver
Copy link
Contributor Author

As said in the summary, I don't expect any performance improvements. That's just to check for regressions (which shouldn't be possible because stack alloc_refs and objects both allocate objects in the same way).

@swift-ci
Copy link
Contributor

swift-ci commented Apr 1, 2020

Build failed before running benchmark.

@zoecarver
Copy link
Contributor Author

@swift-ci please benchmark

@swift-ci
Copy link
Contributor

swift-ci commented Apr 1, 2020

Performance: -O

Regression OLD NEW DELTA RATIO
Hanoi 2110 2430 +15.2% 0.87x
ObjectAllocation 123 135 +9.8% 0.91x (?)
 
Improvement OLD NEW DELTA RATIO
ObserverPartiallyAppliedMethod 1360 820 -39.7% 1.66x
ObserverClosure 1350 820 -39.3% 1.65x

Code size: -O

Improvement OLD NEW DELTA RATIO
Hanoi.o 3120 3056 -2.1% 1.02x
SortArrayInClass.o 2738 2690 -1.8% 1.02x
ObserverForwarderStruct.o 2852 2804 -1.7% 1.02x
RecursiveOwnedParameter.o 2270 2238 -1.4% 1.01x
DictionaryBridge.o 3420 3372 -1.4% 1.01x
ObserverUnappliedMethod.o 5089 5025 -1.3% 1.01x

Performance: -Osize

Regression OLD NEW DELTA RATIO
ObjectiveCBridgeStubToNSDateRef 2120 2320 +9.4% 0.91x (?)
ObjectiveCBridgeStubToNSDate2 330 360 +9.1% 0.92x (?)
ObjectAllocation 135 147 +8.9% 0.92x (?)
 
Improvement OLD NEW DELTA RATIO
FlattenListLoop 3263 2790 -14.5% 1.17x (?)
String.data.LargeUnicode 71 65 -8.5% 1.09x (?)

Code size: -Osize

Performance: -Onone

Regression OLD NEW DELTA RATIO
DataCountSmall 47 51 +8.5% 0.92x (?)
DataSetCountSmall 84 91 +8.3% 0.92x (?)

Code size: -swiftlibs

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 mini
  Model Identifier: Macmini8,1
  Processor Name: 6-Core Intel Core i7
  Processor Speed: 3.2 GHz
  Number of Processors: 1
  Total Number of Cores: 6
  L2 Cache (per Core): 256 KB
  L3 Cache: 12 MB
  Memory: 64 GB

@eeckstein
Copy link
Contributor

See my comment in #30736

@zoecarver
Copy link
Contributor Author

I'm closing this given the discussion in #30787 and because #30810 had significantly better performance improvements.

@zoecarver zoecarver closed this Apr 6, 2020
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.

3 participants