faster-cpython / bench_runner

Code for running pyperformance benchmarks on Github Action runners
BSD 3-Clause "New" or "Revised" License
3 stars 5 forks source link

Support running with Tier 2 switched on #72

Closed mdboom closed 10 months ago

mdboom commented 10 months ago

Currently, to test timings or stats of the Tier 2 interpreter, you need to hardcode that Tier 2 is switched on in the code itself. It would be convenient to have a check mark that would turn this on for a benchmarking run.

However, there are some details to work out:

Should the switch affect just the head commit, or the base commit as well? If it affects the base commit, this will be considerable extra effort, since it's possible different benchmarking runs will request results from the same commit in different configurations (Tier 2 on and off). Currently, it is assumed that the timings/stats from a specific commit will always be the same, so we would need another dimension by which to specify results. Extra complication is that we may also want Tier 2 with or without copy-and-patch, adding a second new dimension. This would be a pretty broad-reaching change -- doable, but only worth doing if this is something people need. Specifically: Do we need to compare Tier 2 of a specific commit against Tier 2 of its base, or is comparing Tier 2 to non-Tier 2 of its base good enough?

Cc: @gvanrossum, @markshannon, @brandtbucher, @ericsnowcurrently, @iritkatriel

gvanrossum commented 10 months ago

Since the current goal is to make Tier 2 faster than Tier 1, and the current status is that Tier 2 is 5-7% slower, I think it's fine to compare Tier 2 with -X uops against Tier 1 without. Once Tier 2 is actually faster we'll likely want to turn it on by default.

IIUC, in Brandt's Justin branch, Tier 2 is currently always on, so maybe we can kick that can down the road?

brandtbucher commented 10 months ago

Agreed that there are kind of two "phases" to this work. Until tier two is faster, we want to compare it to tier one. Once that's accomplished, we'll probably want to start comparing it to itself.

mdboom commented 10 months ago

I've taken the easy path and added a checkbox to turn on Tier 2 on the head commit only -- the base will always be Tier 1 only. Once Tier 2 is the default behavior, it will naturally compare Tier 2 to Tier 2 (and we can remove the checkbox).

mdboom commented 10 months ago

Reopening, as comparing Tier 2 head to Tier 1 base isn't sufficient. Cc @markshannon

mdboom commented 10 months ago

Which combinations do we need?

1) Tier 1 head / Tier 1 base (box unchecked today) 2) Tier 2 head / Tier 1 base (box checked today) 3) Tier 2 head / Tier 2 base 4) Tier 2 head / Tier 1 head

(3) can be accomplished by adding extra metadata / directory structure about build config to runs, and then make sure the flags match when comparing commits. This would remove (2), or we'd need to add another column and compare commits with the same flags and different flags.

(4) would also require that same new metadata, but would also require a third run and an extra comparison (to compare a commit with itself).

A secondary question is whether we automatically always do 3 runs and generate (1), (3) and (4) (A simple UI at the expense of more compute time), or provide some sort of UI to specify what you want, e.g. (but it gets confusing and complicated quickly):

- head: ( ) Tier 1 ( ) Tier 2
- base: ( ) base commit, Tier 1 ( ) base commit, Tier 2, ( ) head commit

I don't want to over-rotate on something that will go away in not too long when Tier 2 is the default.

gvanrossum commented 10 months ago

I hadn’t played with this yet — what’s wrong with the current version? Also, how are you enabling Tier 2? Env var? -Xuops? One line patch to pystate.c? Note the env var was recently renamed to PYTHON_UOPS=1. For running unit tests, only the one line patch is reliable, as there are command line flags (-E) that filter out env vars.

mdboom commented 10 months ago

I hadn’t played with this yet — what’s wrong with the current version?

@markshannon said (in Teams) that comparing Tier 2 head / Tier 1 base wasn't useful.

Also, how are you enabling Tier 2?

With the environment variable. pyperformance provides a method to pass environment variables to the child processes (--inherit-env), but not to pass -X args, so that was just easier. I've confirmed it's working.

markshannon commented 10 months ago

We want to answer two questions with benchmarking tier 2.

mdboom commented 10 months ago

In that case, it seems like the easiest path is to:

  1. Change the checkbox so it runs Tier 2 on head and base (and add the extra metadata to make this possible)
  2. For the weekly runs, measure both Tier 1 and Tier 2, and special case the comparisons on main so that Tier 1 and Tier 2 of the same commit will be compared.
  3. Update the longitudinal plots so that Tier 2 is used instead of Tier 1, if found, for the same commit.
markshannon commented 10 months ago

That sounds good to me