Skip to content

[benchmark] Fix swiftpm based benchmark build on Linux. #20121

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
Merged
merged 1 commit into from
Oct 29, 2018
Merged

[benchmark] Fix swiftpm based benchmark build on Linux. #20121

merged 1 commit into from
Oct 29, 2018

Conversation

gottesmm
Copy link
Contributor

No description provided.

@gottesmm gottesmm requested a review from aciidgh October 29, 2018 08:49
@gottesmm
Copy link
Contributor Author

Please test with following pull request:
swiftlang/swift-integration-tests#36

@swift-ci please test linux platform

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 50b4e0eb99e515918946b739722cef6af6f0ef4a

@gottesmm gottesmm force-pushed the pr-3f72e83daa9956f7052062248db8e4d206f835d9 branch from 50b4e0e to 09f3e2d Compare October 29, 2018 10:11
@gottesmm
Copy link
Contributor Author

Please test with following pull request:
swiftlang/swift-integration-tests#36

@swift-ci please test linux platform

3 similar comments
@gottesmm
Copy link
Contributor Author

Please test with following pull request:
swiftlang/swift-integration-tests#36

@swift-ci please test linux platform

@gottesmm
Copy link
Contributor Author

Please test with following pull request:
swiftlang/swift-integration-tests#36

@swift-ci please test linux platform

@gottesmm
Copy link
Contributor Author

Please test with following pull request:
swiftlang/swift-integration-tests#36

@swift-ci please test linux platform

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 50b4e0eb99e515918946b739722cef6af6f0ef4a

@gottesmm gottesmm force-pushed the pr-3f72e83daa9956f7052062248db8e4d206f835d9 branch from 09f3e2d to a2727a2 Compare October 29, 2018 16:09
@gottesmm
Copy link
Contributor Author

Please test with following pull request:
swiftlang/swift-integration-tests#36

@swift-ci please test linux platform

2 similar comments
@gottesmm
Copy link
Contributor Author

Please test with following pull request:
swiftlang/swift-integration-tests#36

@swift-ci please test linux platform

@gottesmm
Copy link
Contributor Author

Please test with following pull request:
swiftlang/swift-integration-tests#36

@swift-ci please test linux platform

aciidgh
aciidgh reviewed Oct 29, 2018
View reviewed changes
benchmark/Package.swift
@@ -3,13 +3,19 @@
import PackageDescription
import Foundation

var unsupportedTests: Set<String> = ["ObjectiveCNoBridgingStubs"]
#if !os(macOS) && !os(iOS) && !os(watchOS) && !os(tvOS)
Copy link
Contributor

Choose a reason for hiding this comment

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

It'll be better to use canImport(Darwin) or canImport(ObjectiveC)

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - a2727a242e3daddbb1f5367702d66528a6b2a39c

@aciidgh
Copy link
Contributor

aciidgh commented Oct 29, 2018 

/home/buildnode/jenkins/workspace/swift-PR-Linux/branch-master/swift/benchmark/utils/DriverUtils.swift:373:33: error: cannot convert value of type '__rusage_who' to expected argument type '__rusage_who_t' (aka 'Int32')
12:34:08     var u = rusage(); getrusage(RUSAGE_SELF, &u); return u
12:34:08                                 ^~~~~~~~~~~
12:34:08                                            .rawValue
12:34:08 /home/buildnode/jenkins/workspace/swift-PR-Linux/branch-master/swift/benchmark/utils/DriverUtils.swift:385:36: error: cannot convert value of type 'Int' to expected argument type 'Int32'
12:34:08     let pages = { maxRSS / sysconf(_SC_PAGESIZE) }
12:34:08                                    ^~~~~~~~~~~~
12:34:08                                    Int32(      )

@gottesmm
Copy link
Contributor Author

@swift-ci build-toolchain

@gottesmm
Copy link
Contributor Author

@swift-ci build toolchain

@gottesmm gottesmm force-pushed the pr-3f72e83daa9956f7052062248db8e4d206f835d9 branch from a2727a2 to ba7815b Compare October 29, 2018 19:20
@gottesmm
Copy link
Contributor Author

Please test with following pull request:
swiftlang/swift-integration-tests#36

@swift-ci please test linux platform

@gottesmm
Copy link
Contributor Author

@swift-ci smoke benchmark

@gottesmm
Copy link
Contributor Author

Please test with following pull request:
swiftlang/swift-integration-tests#36

@swift-ci please test linux platform

@gottesmm
Copy link
Contributor Author

@swift-ci smoke benchmark

@gottesmm
Copy link
Contributor Author

Please test with following pull request:
swiftlang/swift-integration-tests#36

@swift-ci please test linux platform

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - a2727a242e3daddbb1f5367702d66528a6b2a39c

@swift-ci
Copy link
Contributor

Build comment file:

No performance and code size changes

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: 16 GB

@gottesmm
Copy link
Contributor Author

@swift-ci smoke test os x platform

@gottesmm gottesmm merged commit 6ce58e9 into swiftlang:master Oct 29, 2018
@gottesmm gottesmm deleted the pr-3f72e83daa9956f7052062248db8e4d206f835d9 branch October 29, 2018 20:54
@palimondo
Copy link
Contributor

palimondo commented Oct 31, 2018 •
edited
Loading

Looking at those two compiler errors @aciidb0mb3r points out above… Aren't these bugs in how Glibc imports the RUSAGE_SELF and _SC_PAGESIZE constants? // cc @gottesmm

I mean, what is the point of _SC_PAGESIZE to be platform specific Int, when sysconf expects Int32? One of these seems to be wrong. I've also googled for RUSAGE_SELF.rawValue and found nothing, which again, leads me to believe this is just a Swift specific issue with how these constants get represented in what get's imported as Glibc. I don't have Linux to test on, but this looks really suspicious to me. Where would one file these as bugs?

Granted, there is a genuine issue in that rusage.ru_maxrss is in bytes on Darwin and kilobytes on Linux, which would need a platform specific handling...

@gottesmm
Copy link
Contributor Author

Feel free to file on swift.org.

@palimondo
Copy link
Contributor

Filed SR-9152.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aciidgh aciidgh aciidgh left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gottesmm @swift-ci @aciidgh @palimondo