Skip to content
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

[benchmark] Yield when time slice expires #19011

Merged
merged 3 commits into from Aug 29, 2018

Conversation

palimondo
Copy link
Collaborator

It pays off of to be a nice process and yield the processor at regular intervals, to prevent having measured samples corrupted by preemptive multitasking.

When the scheduled time slice (10ms on Mac OS) is probably about to expire during the next measurement, we voluntarily yield and resume the measurement within a fresh scheduler quantum.

This cooperative approach to multitasking improves the sample quality and robustness of the measurement process.

Marking `Timer` and `SampleRunner` classes as `final` to make sure their methods use static dispatch.
It pays off of to be a nice process and yield the processor at regular intervals, to prevent having measured samples corrupted by preemptive multitasking.

When the scheduled time slice (10ms on Mac OS) is probably about to expire during the next measurement, we voluntarily yield  and resume the measurement within a fresh scheduler quantum.

This cooperative approach to multitasking improves the sample quality and robustness of the measurement process.
@palimondo palimondo changed the title Yield when time slice expires [benchmark] Yield when time slice expires Aug 28, 2018
@palimondo
Copy link
Collaborator Author

Please review @eeckstein (@graydon ?) 🙏

@graydon
Copy link
Contributor

graydon commented Aug 28, 2018

Clever! Looks good to me.

@palimondo
Copy link
Collaborator Author

I'm unsure if I should hide this behind option or possibly make the schedulerQuantum into a configurable argument, so that we might better test the effect of this on sample quality. It doesn't do anything noticeable on a calm machine, but the point is to make the measurement more resilient to noise on a contended machine, as I'd ultimately like to make benchmarking parallel. I did remeasure the complete dataset from my report, but a lot has changed in the meantime and most of individual benchmarks are not directly comparable anymore.

@palimondo
Copy link
Collaborator Author

palimondo commented Aug 28, 2018

To illustrate the effect of this change on a contended machine, this is a before and after example:
img_0576
img_0577
It’s not that the measurement took this much less, it’s just that those interrupts were not included in our measurements, because we have voluntarily yielded. Note: d Series is measured with 3 concurrent benchmark processes on a 2 core machine.

Copy link
Member

@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.

lgtm!

@eeckstein
Copy link
Member

@swift-ci smoke benchmark

@eeckstein
Copy link
Member

@swift-ci smoke test

@swift-ci
Copy link
Collaborator

Build comment file:

Build failed before running benchmark.


@shahmishal
Copy link
Member

@swift-ci smoke test

@shahmishal
Copy link
Member

@swift-ci smoke benchmark

@shahmishal
Copy link
Member

@swift-ci benchmark

@swift-ci
Copy link
Collaborator

Build comment file:

Build failed before running benchmark.


1 similar comment
@swift-ci
Copy link
Collaborator

Build comment file:

Build failed before running benchmark.


@palimondo
Copy link
Collaborator Author

palimondo commented Aug 29, 2018

@shahmishal Any idea why are these benchmark builds failing?

@shahmishal
Copy link
Member

This might be an issue on the master branch, not related to your changes.

<unknown>:0: error: unable to execute command: Segmentation fault: 11
01:56:25.197 <unknown>:0: error: compile command failed due to signal 11 (use -v to see invocation)
01:56:25.946 [932/1590] Generating /Users/buildnode/jenkins/workspace/swift-PR-osx-perf/Ninja-Release/swift-macosx-x86_64/./lib/swift/macosx/x86_64/Swift.swiftmodule
01:56:25.946 FAILED: lib/swift/macosx/x86_64/Swift.swiftmodule lib/swift/macosx/x86_64/Swift.swiftdoc lib/swift/macosx/x86_64/Swift.swiftinterface 
01:56:25.946 cd /Users/buildnode/jenkins/workspace/swift-PR-osx-perf/Ninja-Release/swift-macosx-x86_64/stdlib/public/core && /Applications/CMake.app/Contents/bin/cmake -E remove -f /Users/buildnode/jenkins/workspace/swift-PR-osx-perf/Ninja-Release/swift-macosx-x86_64/./lib/swift/macosx/x86_64/Swift.swiftmodule && /Applications/CMake.app/Contents/bin/cmake -E remove -f /Users/buildnode/jenkins/workspace/swift-PR-osx-perf/Ninja-Release/swift-macosx-x86_64/./lib/swift/macosx/x86_64/Swift.swiftdoc && /Applications/CMake.app/Contents/bin/cmake -E remove -f /Users/buildnode/jenkins/workspace/swift-PR-osx-perf/Ninja-Release/swift-macosx-x86_64/./lib/swift/macosx/x86_64/Swift.swiftinterface && /usr/bin/python /Users/buildnode/jenkins/workspace/swift-PR-osx-perf/swift/utils/line-directive @/Users/buildnode/jenkins/workspace/swift-PR-osx-perf/Ninja-Release/swift-macosx-x86_64/stdlib/public/core/aG1Cy.txt -- /Users/buildnode/jenkins/workspace/swift-PR-osx-perf/Ninja-Release/swift-macosx-x86_64/./bin/swiftc -emit-module -o /Users/buildnode/jenkins/workspace/swift-PR-osx-perf/Ninja-Release/swift-macosx-x86_64/./lib/swift/macosx/x86_64/Swift.swiftmodule -experimental-emit-interface -sdk /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk -target x86_64-apple-macosx10.9 -resource-dir /Users/buildnode/jenkins/workspace/swift-PR-osx-perf/Ninja-Release/swift-macosx-x86_64/./lib/swift -F /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/../../../Developer/Library/Frameworks -O -I /Users/buildnode/jenkins/workspace/swift-PR-osx-perf/Ninja-Release/swift-macosx-x86_64/./lib/swift/macosx/x86_64 -module-cache-path /Users/buildnode/jenkins/workspace/swift-PR-osx-perf/Ninja-Release/swift-macosx-x86_64/./module-cache -no-link-objc-runtime -Xfrontend -enable-resilience -nostdimport -parse-stdlib -module-name Swift -Xfrontend -group-info-path -Xfrontend /Users/buildnode/jenkins/workspace/swift-PR-osx-perf/swift/stdlib/public/core/GroupInfo.json -swift-version 5 -warn-swift3-objc-inference-complete -Xfrontend -verify-syntax-tree -Xllvm -sil-inline-generics -Xllvm -sil-partial-specialization -Xfrontend -enable-sil-ownership -Xcc -DswiftCore_EXPORTS -module-link-name swiftCore -force-single-frontend-invocation -Xcc -D__SWIFT_CURRENT_DYLIB=swiftCore -parse-as-library @/Users/buildnode/jenkins/workspace/swift-PR-osx-perf/Ninja-Release/swift-macosx-x86_64/stdlib/public/core/aG1Cy.txt
01:56:25.946 Stack dump:
01:56:25.946 0.	Program arguments: /Users/buildnode/jenkins/workspace/swift-PR-osx-perf/Ninja-Release/swift-macosx-x86_64/bin/swift -frontend -emit-module -filelist /var/folders/_8/79jmzf2142z2xydc_01btlx00000gn/T/sources-a2ba25 -supplementary-output-file-map /var/folders/_8/79jmzf2142z2xydc_01btlx00000gn/T/supplementaryOutputs-2a8957 -disable-objc-attr-requires-foundation-module -target x86_64-apple-macosx10.9 -enable-objc-interop -sdk /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk -I /Users/buildnode/jenkins/workspace/swift-PR-osx-perf/Ninja-Release/swift-macosx-x86_64/./lib/swift/macosx/x86_64 -F /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/../../../Developer/Library/Frameworks -warn-swift3-objc-inference-complete -module-cache-path /Users/buildnode/jenkins/workspace/swift-PR-osx-perf/Ninja-Release/swift-macosx-x86_64/./module-cache -module-link-name swiftCore -nostdimport -parse-stdlib -resource-dir /Users/buildnode/jenkins/workspace/swift-PR-osx-perf/Ninja-Release/swift-macosx-x86_64/./lib/swift -swift-version 5 -O -enable-resilience -group-info-path /Users/buildnode/jenkins/workspace/swift-PR-osx-perf/swift/stdlib/public/core/GroupInfo.json -verify-syntax-tree -enable-sil-ownership -Xllvm -sil-inline-generics -Xllvm -sil-partial-specialization -Xcc -DswiftCore_EXPORTS -Xcc -D__SWIFT_CURRENT_DYLIB=swiftCore -parse-as-library -module-name Swift -o /Users/buildnode/jenkins/workspace/swift-PR-osx-perf/Ninja-Release/swift-macosx-x86_64/./lib/swift/macosx/x86_64/Swift.swiftmodule 
01:56:25.946 1.	Contents of /var/folders/_8/79jmzf2142z2xydc_01btlx00000gn/T/sources-a2ba25:

@palimondo
Copy link
Collaborator Author

palimondo commented Aug 29, 2018

:-) I figured that much… I just thought there is some kind of a process for reverting a commit that breaks the build that would restore master to working order.… (in less than 6 hours since this started)

Also it's a bit strange that smoke test passed and the issue is only with benchmark build...

@eeckstein
Copy link
Member

@swift-ci benchmark

@swift-ci
Copy link
Collaborator

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

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.

None yet

5 participants