Allow perf harness to compare multiple revisions of SwiftProtobuf #426
Conversation
| Additionally, you can specify "all" to run the harness multiple | ||
| times with all of the (non-repeated) field types listed above. | ||
| ("all" is not compatible with revision comparisons.) |
There was a problem hiding this comment.
Especially for revision comparisons, should we look at adding a "mixed" or something that is a message with say two dozen fields of differing types and also uses messages within messages to get some depth?
There was a problem hiding this comment.
There's a TODO in there to do this :) So yes.
Performance/perf_runner.sh
Outdated
|
|
||
| readonly field_count="$1"; shift | ||
| if [[ "$1" == "all" ]]; then | ||
| # if [[ "$comparison" != "c++" ]]; then |
There was a problem hiding this comment.
delete since it is commented out?
|
|
||
| # Check to see if we have past results from that commit cached. If so, we | ||
| # don't need to run it again. | ||
| if [[ ! -f "$commit_results" ]]; then |
There was a problem hiding this comment.
What if it was testing a different field count/etc.?
There was a problem hiding this comment.
The filename includes the field type and count in addition to the hash.
|
|
||
| # Check out the commit to a temporary directory and create its _generated | ||
| # directory. (Results will still go in the working tree.) | ||
| tmp_checkout="$(mktemp -d -t swiftprotoperf)" |
There was a problem hiding this comment.
do you want an exit hook to delete this directory?
Early on, set up a hook:
CLEANUP_WHEN_DONE=()
function cleanup() {
rm -rf "${CLEANUP_WHEN_DONE[@]}"
}
trap cleanup EXIT HUP INT QUIT TERMThe this directory creation becomes:
tmp_checkout="$(mktemp -d -t swiftprotoperf)"
CLEANUP_WHEN_DONE+=("$tmp_checkout")Then no mater now the script dies/exits (unless it is an exec), it will ensure everything added to CLEANUP_WHEN_DONE gets removed at the end.
There was a problem hiding this comment.
I was trying to think of a clean way to do that but my bash-fu isn't that strong—I like your approach.
2 participants
Whew. I got inspired yesterday, and I should have done this a long time ago; it wasn't as bad as I thought it would be.
The harness now takes
-coptions that let you specify git revisions that should be benchmarked and compared to the current working tree. They can be specified as anything the git tools allow—I imagine branch names (probablymaster) will be most useful but you can also use commit hashes and whatnot.-calso accepts the stringc++, which means compare against C++ (the old behavior, and also what happens if you don't use any-coptions). You can specify-cmultiple times, each one becomes a series in the plot, and the current working tree is always last; we're not limited to two anymore (we probably won't use this very much, unless we want to compare, say,master, the working tree, and C++ all at once).This works by checking out the local repo history at the given revisions into temporary locations and building the harnesses there. This means it uses the generator and the runtime at that revision—so even if you're changing generator code, you end up generating the right thing for each revision and it just works.
It should be noted that this will only work for commits going forward from the time this is merged; because of changes I made to the harness generating scripts to separate the generation from the running (to isolate API changes), the harness makes assumptions about the scripts that will be available in the checkout.
I tested it by adding a
usleep(1)in the binary decoder (I can't account for the seemingly nontrivial noise elsewhere in the two branches, though):This was generated with the command line: