Skip to content

[cxx-interop] Add support for benchmarks with C++ interop. #32721

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

Merged
merged 1 commit into from
Nov 20, 2020

Conversation

zoecarver
Copy link
Contributor

  • Adds support for benchmarks that use C++ modules.
  • Adds "CreateObjects" benchmark that creates C++ objects.

Based on #32715.

@zoecarver zoecarver added the c++ interop Feature: Interoperability with C++ label Jul 6, 2020
@zoecarver zoecarver requested review from gribozavr and MForster July 6, 2020 16:38
Copy link
Contributor

@MForster MForster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to contain a lot of unrelated changes...

@zoecarver
Copy link
Contributor Author

This seems to contain a lot of unrelated changes...

Yes, this PR is based on #32715 so all the changes from that PR are here too (just so it compiles).

@zoecarver zoecarver force-pushed the cxx/benchmark/create-objects branch from 4e31904 to 0a88a45 Compare July 7, 2020 16:49
Copy link
Contributor

@MForster MForster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(after the base pull request has been merged)

@zoecarver
Copy link
Contributor Author

@MForster sounds good. Thanks!

Copy link
Contributor

@gribozavr gribozavr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zoecarver This PR is adding benchmark/.swiftpm/xcode/package.xcworkspace/xcuserdata/zoe.xcuserdatad/UserInterfaceState.xcuserstate, I think that's not intentional.

@@ -817,6 +818,17 @@ class IRGenSILFunction :
SILType SILTy, const SILDebugScope *DS,
VarDecl *VarDecl, SILDebugVariable VarInfo,
IndirectionKind Indirection = DirectValue) {
// TODO: fix demangling for C++ types.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a separate PR for these changes to the compiler?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, there's not. I'll make one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here it is: #32815.

@zoecarver
Copy link
Contributor Author

This PR is adding benchmark/.swiftpm/xcode/package.xcworkspace/xcuserdata/zoe.xcuserdatad/UserInterfaceState.xcuserstate, I think that's not intentional.

Oops, you're right. Xcode adds that when I open the benchmarks/ directory. Forgot to remove it last time I committed.

@gribozavr
Copy link
Contributor

LGTM! Please rebase and merge after #32815 goes in.

@zoecarver zoecarver force-pushed the cxx/benchmark/create-objects branch 2 times, most recently from d488500 to 2e389d6 Compare July 23, 2020 22:51
@zoecarver
Copy link
Contributor Author

@swift-ci please test and merge

@zoecarver
Copy link
Contributor Author

@swift-ci please smoke test and merge

1 similar comment
@zoecarver
Copy link
Contributor Author

@swift-ci please smoke test and merge

@zoecarver
Copy link
Contributor Author

@swift-ci please test Linux platform.

@zoecarver
Copy link
Contributor Author

@swift-ci please test Linux platform

@zoecarver
Copy link
Contributor Author

@swift-ci please benchmark

@zoecarver
Copy link
Contributor Author

@swift-ci please test windows platform.

@zoecarver
Copy link
Contributor Author

@swift-ci please benchmark

@zoecarver
Copy link
Contributor Author

@swift-ci please benchmark.

@zoecarver
Copy link
Contributor Author

@swift-ci please benchmark

1 similar comment
@zoecarver
Copy link
Contributor Author

@swift-ci please benchmark

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 2e389d641f88977b77cf14ea612fa7d479499eb0

@swift-ci
Copy link
Contributor

Performance: -O

Regression OLD NEW DELTA RATIO
NSStringConversion.MutableCopy.Rebridge.Long 996 1072 +7.6% 0.93x (?)
 
Added MIN MAX MEAN MAX_RSS
CreateObjects 1 1 1

Code size: -O

Performance: -Osize

Regression OLD NEW DELTA RATIO
FlattenListFlatMap 4123 6572 +59.4% 0.63x (?)
ObjectiveCBridgeStubFromNSDate 5980 7150 +19.6% 0.84x (?)
CharacterLiteralsLarge 100 111 +11.0% 0.90x (?)
DataSetCountMedium 320 350 +9.4% 0.91x (?)
 
Improvement OLD NEW DELTA RATIO
ObjectiveCBridgeStubDateAccess 285 257 -9.8% 1.11x
ObjectiveCBridgeStubToNSDateRef 3900 3520 -9.7% 1.11x (?)
 
Added MIN MAX MEAN MAX_RSS
CreateObjects 2 2 2

Code size: -Osize

Performance: -Onone

Regression OLD NEW DELTA RATIO
ObjectiveCBridgeStubFromNSDate 5840 6860 +17.5% 0.85x (?)
StringBuilderWithLongSubstring 2910 3200 +10.0% 0.91x (?)
 
Improvement OLD NEW DELTA RATIO
CStringLongAscii 321 288 -10.3% 1.11x (?)
Hanoi 13860 12770 -7.9% 1.09x (?)
 
Added MIN MAX MEAN MAX_RSS
CreateObjects 190 203 195

Code size: -swiftlibs

Benchmark Check Report
⚠️ CreateObjects execution took 1 μs.
Increase the workload of CreateObjects to be more than 20 μs.
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

@zoecarver zoecarver force-pushed the cxx/benchmark/create-objects branch 2 times, most recently from 28ee166 to f79f5be Compare September 9, 2020 03:53
@zoecarver
Copy link
Contributor Author

@swift-ci please benchmark

@zoecarver
Copy link
Contributor Author

@swift-ci please smoke test

@swift-ci
Copy link
Contributor

swift-ci commented Sep 9, 2020

Performance: -O

Regression OLD NEW DELTA RATIO
ArrayPlusEqualSingleElementCollection 423 470 +11.1% 0.90x (?)
DataSubscriptMedium 37 41 +10.8% 0.90x (?)
 
Improvement OLD NEW DELTA RATIO
ObjectiveCBridgeStubDateAccess 152 130 -14.5% 1.17x
 
Added MIN MAX MEAN MAX_RSS
CreateObjects 11 11 11

Code size: -O

Performance: -Osize

Regression OLD NEW DELTA RATIO
Dictionary4 157 197 +25.5% 0.80x (?)
StringWalk 1640 1800 +9.8% 0.91x (?)
 
Improvement OLD NEW DELTA RATIO
ObjectiveCBridgeStubDateAccess 152 130 -14.5% 1.17x
CharacterLiteralsLarge 71 63 -11.3% 1.13x (?)
CharacterLiteralsSmall 228 211 -7.5% 1.08x (?)
ArrayLiteral2 116 108 -6.9% 1.07x (?)
 
Added MIN MAX MEAN MAX_RSS
CreateObjects 11 11 11

Code size: -Osize

Performance: -Onone

Added MIN MAX MEAN MAX_RSS
CreateObjects 1391 1459 1429

Code size: -swiftlibs

Benchmark Check Report
⚠️ CreateObjects execution took 11 μs.
Increase the workload of CreateObjects to be more than 20 μs.
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

tags: [.validation, .bridging])

@inline(never)
public func run_CreateObjects(_ N: Int) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this benchmark is just there to validate that C++ support is compiling fine.
I think it makes sense to disable this benchmark from the regular run, because it's not doing anything interesting.
Can you add the ".skip" benchmark tag?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I'd like to test the performance of creating C++ objects. For example, I expect that this will be much slower after #30630 (when we actually have to call the constructor instead of just allocating an empty object).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Makes sense

@zoecarver zoecarver force-pushed the cxx/benchmark/create-objects branch from f79f5be to c1235c2 Compare September 10, 2020 19:41
@zoecarver
Copy link
Contributor Author

@swift-ci please smoke test Linux platform.

@zoecarver
Copy link
Contributor Author

@swift-ci please smoke test Linux platform

@zoecarver zoecarver force-pushed the cxx/benchmark/create-objects branch from c1235c2 to 19b875a Compare September 10, 2020 23:44
@zoecarver
Copy link
Contributor Author

@swift-ci please smoke test Linux platform

1 similar comment
@zoecarver
Copy link
Contributor Author

@swift-ci please smoke test Linux platform

@zoecarver
Copy link
Contributor Author

The build failure appears to be unrelated.

@zoecarver
Copy link
Contributor Author

@swift-ci please smoke test Linux platform.

@shahmishal
Copy link
Member

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)

@zoecarver zoecarver changed the base branch from master to main October 2, 2020 04:03
* Adds support for benchmarks that use C++ modules.
* Adds "CreateObjects" benchmark that creates C++ objects.
@zoecarver zoecarver force-pushed the cxx/benchmark/create-objects branch from 19b875a to 01b12cf Compare October 7, 2020 02:39
@zoecarver
Copy link
Contributor Author

@swift-ci please smoke test Linux platform.

@zoecarver
Copy link
Contributor Author

Failed because of an unrelated issue.

@swift-ci please smoke test Linux platform.

@MaxDesiatov
Copy link
Contributor

swiftlang/swift-corelibs-foundation#2895
@swift-ci please smoke test

@zoecarver
Copy link
Contributor Author

@swift-ci please test.

1 similar comment
@zoecarver
Copy link
Contributor Author

@swift-ci please test.

@zoecarver zoecarver merged commit aa04872 into swiftlang:main Nov 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ interop Feature: Interoperability with C++
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants