Skip to content

[nfc] [benchmark] [cxx-interop] Add "GlobalVars" benchmark to test static variables. #35318

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

[nfc] [benchmark] [cxx-interop] Add "GlobalVars" benchmark to test static variables. #35318

wants to merge 1 commit into from

Conversation

zoecarver
Copy link
Contributor

Tests performance of loading static C++ variables from Swift. Tests both constexpr and non-constexpr statics.

Refs #35311.

@zoecarver zoecarver requested review from compnerd and hlopko January 8, 2021 20:40
@zoecarver
Copy link
Contributor Author

@swift-ci please benchmark.

1 similar comment
@zoecarver
Copy link
Contributor Author

@swift-ci please benchmark.

@zoecarver zoecarver added the c++ interop Feature: Interoperability with C++ label Jan 8, 2021
compnerd
compnerd reviewed Jan 8, 2021
View reviewed changes
@zoecarver zoecarver force-pushed the cxx/benchmark-global-vars branch from ef94a8d to 7843859 Compare January 8, 2021 21:38
@zoecarver
Copy link
Contributor Author

@swift-ci please benchmark.

@swift-ci
Copy link
Contributor

swift-ci commented Jan 9, 2021

Build failed before running benchmark.

@zoecarver zoecarver force-pushed the cxx/benchmark-global-vars branch from 7843859 to dff3e4e Compare January 9, 2021 01:04
@zoecarver
[nfc] [benchmark] [cxx-interop] Add "GlobalVars" benchmark to test gl…
9924c98 
…obal static vars.

Tests performance of loading static C++ variables from Swift. Tests  both constexpr and non-constexpr statics.
zoecarver
zoecarver commented Jan 9, 2021
View reviewed changes
benchmark/utils/CxxTests/GlobalVars.h Outdated
static BigObject globalBigObject = BigObject();
static constexpr BigObject globalConstexprBigObject = BigObject();

struct HasStaticConstexprMembers {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Funny enough, these tests actually fail because of undefined symbols without #35311.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Planning to 1) land this 2) test #35311 3) add these tests to #35311 4) land #35311.

@zoecarver zoecarver force-pushed the cxx/benchmark-global-vars branch from dff3e4e to 9924c98 Compare January 9, 2021 01:57
@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

swift-ci commented Jan 9, 2021

Performance: -O

Regression OLD NEW DELTA RATIO
CharacterLiteralsLarge 87 97 +11.5% 0.90x
StringToDataLargeUnicode 3200 3500 +9.4% 0.91x (?)
 
Improvement OLD NEW DELTA RATIO
Data.append.Sequence.64kB.Count.RE.I 38 35 -7.9% 1.09x (?)
 
Added MIN MAX MEAN MAX_RSS
GlobalVars 358 361 359 —

Code size: -O

Performance: -Osize

Regression OLD NEW DELTA RATIO
Breadcrumbs.MutatedIdxToUTF16.ASCII 3 4 +33.3% 0.75x
ObjectiveCBridgeStubToNSStringRef 102 112 +9.8% 0.91x (?)
 
Added MIN MAX MEAN MAX_RSS
GlobalVars 352 354 353 —

Code size: -Osize

Performance: -Onone

Regression OLD NEW DELTA RATIO
DataToStringSmall 4700 5200 +10.6% 0.90x (?)
 
Added MIN MAX MEAN MAX_RSS
GlobalVars 14441 15062 14702 —

Code size: -swiftlibs

✅ Benchmark Check Report
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: 8-Core Intel Xeon E5
  Processor Speed: 3 GHz
  Number of Processors: 1
  Total Number of Cores: 8
  L2 Cache (per Core): 256 KB
  L3 Cache: 25 MB
  Memory: 64 GB

@zoecarver
Copy link
Contributor Author

I'm going to go ahead and land this once the tests pass.

@swift-ci please smoke test.

@zoecarver
Copy link
Contributor Author

@swift-ci please smoke test macOS.

@zoecarver
Copy link
Contributor Author

@swift-ci please smoke test Windows.

1 similar comment
@zoecarver
Copy link
Contributor Author

@swift-ci please smoke test Windows.

@zoecarver
Copy link
Contributor Author

@swift-ci please test Windows.

@zoecarver
Copy link
Contributor Author

@swift-ci please smoke test Linux.

@zoecarver
Copy link
Contributor Author

@swift-ci please test Windows.

1 similar comment
@zoecarver
Copy link
Contributor Author

@swift-ci please test Windows.

@zoecarver
Copy link
Contributor Author

Keep getting Java errors with the Linux CI.

@swift-ci please smoke test Linux.

@zoecarver
Copy link
Contributor Author

@swift-ci please smoke test Linux.

1 similar comment
@zoecarver
Copy link
Contributor Author

@swift-ci please smoke test Linux.

@eeckstein eeckstein self-requested a review January 11, 2021 15:47
eeckstein
eeckstein reviewed Jan 11, 2021
View reviewed changes
Copy link
Contributor

@eeckstein eeckstein left a 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 should test that the code generation for loading globals is optimal.
For such trivial things, it's often better to add a lit test which e.g. checks if the SIL (or LLVM IR) looks optimal.

@zoecarver
Copy link
Contributor Author

I assume this benchmark should test that the code generation for loading globals is optimal.
For such trivial things, it's often better to add a lit test which e.g. checks if the SIL (or LLVM IR) looks optimal.

I think you're right. Especially now that I've had to remove about two-thirds of the tests. OK, I'll close it.

@zoecarver zoecarver closed this Jan 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@compnerd compnerd compnerd left review comments

@eeckstein eeckstein eeckstein left review comments

@hlopko hlopko Awaiting requested review from hlopko

Assignees
No one assigned
Labels
c++ interop Feature: Interoperability with C++
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@zoecarver @swift-ci @compnerd @eeckstein