-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[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
[cxx-interop] Add support for benchmarks with C++ interop. #32721
Conversation
There was a problem hiding this 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...
Yes, this PR is based on #32715 so all the changes from that PR are here too (just so it compiles). |
4e31904
to
0a88a45
Compare
There was a problem hiding this 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)
@MForster sounds good. Thanks! |
There was a problem hiding this 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.
lib/IRGen/IRGenSIL.cpp
Outdated
@@ -817,6 +818,17 @@ class IRGenSILFunction : | |||
SILType SILTy, const SILDebugScope *DS, | |||
VarDecl *VarDecl, SILDebugVariable VarInfo, | |||
IndirectionKind Indirection = DirectValue) { | |||
// TODO: fix demangling for C++ types. |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here it is: #32815.
Oops, you're right. Xcode adds that when I open the |
0a88a45
to
059c6b0
Compare
059c6b0
to
df25b96
Compare
LGTM! Please rebase and merge after #32815 goes in. |
d488500
to
2e389d6
Compare
@swift-ci please test and merge |
@swift-ci please smoke test and merge |
1 similar comment
@swift-ci please smoke test and merge |
@swift-ci please test Linux platform. |
@swift-ci please test Linux platform |
@swift-ci please benchmark |
@swift-ci please test windows platform. |
@swift-ci please benchmark |
@swift-ci please benchmark. |
@swift-ci please benchmark |
1 similar comment
@swift-ci please benchmark |
Build failed |
Performance: -O
Code size: -OPerformance: -Osize
Code size: -OsizePerformance: -Onone
Code size: -swiftlibs
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 Noise: Sometimes the performance results (not code size!) contain false Hardware Overview
|
28ee166
to
f79f5be
Compare
@swift-ci please benchmark |
@swift-ci please smoke test |
Performance: -O
Code size: -OPerformance: -Osize
Code size: -OsizePerformance: -Onone
Code size: -swiftlibs
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 Noise: Sometimes the performance results (not code size!) contain false Hardware Overview
|
tags: [.validation, .bridging]) | ||
|
||
@inline(never) | ||
public func run_CreateObjects(_ N: Int) { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Makes sense
f79f5be
to
c1235c2
Compare
@swift-ci please smoke test Linux platform. |
@swift-ci please smoke test Linux platform |
c1235c2
to
19b875a
Compare
@swift-ci please smoke test Linux platform |
1 similar comment
@swift-ci please smoke test Linux platform |
The build failure appears to be unrelated. |
@swift-ci please smoke test Linux platform. |
* Adds support for benchmarks that use C++ modules. * Adds "CreateObjects" benchmark that creates C++ objects.
19b875a
to
01b12cf
Compare
@swift-ci please smoke test Linux platform. |
Failed because of an unrelated issue. @swift-ci please smoke test Linux platform. |
swiftlang/swift-corelibs-foundation#2895 |
@swift-ci please test. |
1 similar comment
@swift-ci please test. |
Based on #32715.