Skip to content

Conversation

@aalexand
Copy link
Collaborator

@aalexand aalexand commented Jul 21, 2017

TestHttpsInsecure test runs a profiling session and checks that the
results contain expected function name. On OSX prior to 10.11 El Capitan
the SIGPROF signal is delivered to the process rather than to the thread
which skews the results enough for the test to fail.

Relax the check when running on OSX and other operating systems known to
have issues with the profiling signal (the list is taken from
src/runtime/pprof/pprof_test.go in Go source).

Also add the multiple variants of OSX to the continuous testing.

Fixes #146 and fixes #156.

TestHttpsInsecure test runs a profiling session and checks that the
results contain expected function name. On OSX prior to 10.11 El Capitan
the SIGPROF signal is delivered to the process rather than to the thread
which skews the results enough for the test to fail.

Relax the check when running on OSX and other operating systems known to
have issues with the profiling signal (the list is taken from
src/runtime/pprof/pprof_test.go in Go source).

Also add the multiple variants of OSX to the continuous testing.

Fixes google#146 and google#156.
@codecov-io
Copy link

codecov-io commented Jul 21, 2017

Codecov Report

Merging #171 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #171   +/-   ##
=======================================
  Coverage   53.45%   53.45%           
=======================================
  Files          32       32           
  Lines        6181     6181           
=======================================
  Hits         3304     3304           
  Misses       2521     2521           
  Partials      356      356

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 92a0537...14747ff. Read the comment docs.

deadline := time.Now().Add(5 * time.Second)
for time.Now().Before(deadline) {
// Simulate a hotspot function.
for i := 0; i < 1e8; i++ {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the empty loop? Doesn't the Go compiler remove it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have added a comment. The purpose is to ensure the samples mostly land onto the loop. As far as I can tell Go compiler doesn't elide empty loops now.

@aalexand0:tmp$ cat test.go
package main

func main() {
        for i := 0; i < 16; i++ {
        }
}
@aalexand0:tmp$ go build test.go
@aalexand0:tmp$ objdump -d test | grep -A10 "<main.main>:"
00000000004511c0 <main.main>:
  4511c0:       31 c0                   xor    %eax,%eax
  4511c2:       eb 03                   jmp    4511c7 <main.main+0x7>
  4511c4:       48 ff c0                inc    %rax
  4511c7:       48 83 f8 10             cmp    $0x10,%rax
  4511cb:       7c f7                   jl     4511c4 <main.main+0x4>
  4511cd:       c3                      retq   
  4511ce:       cc                      int3   
  4511cf:       cc                      int3   

00000000004511d0 <main.init>:

@aalexand aalexand merged commit 2121546 into google:master Jul 26, 2017
gmarin13 pushed a commit to gmarin13/pprof that referenced this pull request Dec 17, 2020
* Make TestHttpsInsecure test reliably passing on Darwin.

TestHttpsInsecure test runs a profiling session and checks that the
results contain expected function name. On OSX prior to 10.11 El Capitan
the SIGPROF signal is delivered to the process rather than to the thread
which skews the results enough for the test to fail.

Relax the check when running on OSX and other operating systems known to
have issues with the profiling signal (the list is taken from
src/runtime/pprof/pprof_test.go in Go source).

Also add the multiple variants of OSX to the continuous testing.

Fixes google#146 and google#156.

* Document the motivation for the empty loop.
@aalexand aalexand deleted the osx branch January 11, 2023 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make Travis CI run against several OSX versions TestHttpsInsecure failures on certain darwin builders

5 participants