[benchmark] BenchmarkDriver check --markdown

[palimondo]

Added --markdown flag for the check command to output the BenchmarkDoctor’s report in the Markdown format (as used by swift-ci on GitHub).

[palimondo]

@swift-ci please test

[palimondo]

@eeckstein Please review 🙏

[eeckstein]

@swift-ci benchmark

[swift-ci]

Build comment file:

---

[eeckstein]

Please look at the benchmark build log. There is a crash in the script.

[palimondo]

It's great that test also runs benchmarks using run_smoke_bench! Automated safety net FTW 👍

Update: Oh! It didn't 🤦‍♂️ You did… We don't do that because it requires building 2x?

[palimondo]

@swift-ci please test

[swift-ci]

Build failed
Swift Test OS X Platform
Git Sha - 46f94d7

[eeckstein]

It does not. I started a benchmark run

[eeckstein]

@swift-ci benchmark

[eeckstein]

And, as it's not a compiler change, it's sufficient if you do a smoke test for this.

[palimondo]

Hmm… I ran full test, because I wanted CI to run through bencmark's lit tests, which are not executed during smoke test (?). Can we at least make those run in smoke, do that I don't need to invoke full test for that reason?

[eeckstein]

They should run with smoke test, because they are in swift/test. But maybe I'm missing something here.

[palimondo]

I think smoke test doesn't build benchmark binaries for some reason. The unit tests for benchmark scripts get executed (pure python), but not the Benchmark_O.test.md and Benchmark_Driver.test-sh because they both have REQUIRES: benchmark in them.

[palimondo]

But none of them would have caught this bug…😔

It didn't occur to me I also need to adjust run_smoke_bench when I've modified the args Stub to make the other test's pass... my bad. But the workaround in b831f93 should do the trick as well.

[swift-ci]

Build comment file:

Performance: -O

TEST	OLD	NEW	DELTA	RATIO
Regression
ObjectiveCBridgeFromNSArrayAnyObjectToString	47109	53458	+13.5%	0.88x

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

--------------

[palimondo]

@eeckstein Can we make smoke test build the benchmark binary?
(I don't know how to make that change...)

[eeckstein]

That needs negotiation with @shahmishal. We are trying very hard to keep the smoke test time down to a minimum.

[shahmishal]

@palimondo / @eeckstein How much extra time are we looking at building benchmark binary?

[eeckstein]

When we just build a single executable, e.g. Benchmark_O (and not the Osize and Onone variants), it's about 5min user time on my machine. + 4 sec to run it (what we should do, because this gives more test coverage)

[palimondo]

Really don't know. I'm running my local builds on 2008 MBP…