Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

The Great Merging #2

merged 46 commits into from Jan 8, 2012


None yet
2 participants

arc commented Jan 8, 2012

I've now folded all of cmp_pq's features into bmark_pq. Well, almost all — see the comment starting on line 75 of bmark_pq. Any thoughts about how to deal with that problem are welcome.

Also, I fear I'm now the blameworthy person for more than half of the 1330 committed lines in this repo. Sorry about that.

arc added some commits Jan 7, 2012

@arc arc Avoid implicit function imports e5be3b1
@arc arc Fix stupid sort/unique bug a607150
@arc arc bin/bmark_pq: stop on invalid options ff21364
@arc arc bin/bmark_pq: stop on excess non-option arguments 3fe7f10
@arc arc bin/bmark_pq: make `--help` and `-?` options work
Also, stop advertising the `-h` synonym.  I find it bletcherous to steal one
of the precious single-letter options for something that has at least two
better spellings anyway.  Failing to advertise its existence means that
reusing it could at least be considered if a better use is found for this
option in the future.
@arc arc bin/bmark_pq: refactor calculation of the timeout 7cfe235
@arc arc bin/bmark_pq: don't send error messages to stdout acc90b6
@arc arc Refactor run_all_benchmarks()
I find that this change clarifies its relationship to run_benchmark().
@arc arc Export all_benchmarks() and all_tested_modules()
They're used in user-space code, not just tests.
@arc arc Delete Benchmark::PriorityQueue::module_is_tested() function
It was only used in tests, not exported, and the module does export a
function which provides enough information to answer the same question.
That does come at a small cost in convenience, but only the test must pay
that price, and the benefit is that we get a smaller API.
@arc arc Delete run_all_benchmarks()
Incredibly, despite the fact that the tests and documentation get more
complicated, and the fact that the loop is repeated in several callers, this
actually gives us less code overall.  So I think it's a win on that basis
alone.  Furthermore, as we mine bin/bmark_pq and bin/cmp_pq for extractable
similarities, I think it'll be awkward to embed that top-level loop in an
exported function, rather than letting callers loop over the right things
@arc arc cmp_pq: fix typo in usage message 25cccb2
@arc arc Delete unused import 9e0a529
@arc arc Don't leak classes' imported functions as methods dd507dd
@arc arc Delete duplicate method definitions 69e8a15
@arc arc Move methods to correct place
The `pop_highest_ordered_mod3` and `pop_highest_random_mod3` shim methods
should live in the PopsHighest role, not the shim base class, since they
need the `pop_highest` method already required by that role.
@arc arc Refactor the `benchmark_code` method modifiers
Seems less repetitive this way, despite being slightly longer by line count.
@arc arc Add missing `pop_highest_*_mod3` tasks
The methods existed, but they weren't being returned by `benchmark_code`, so
nothing would try to run them.
@arc arc New `pop_lowest_*_mod3` benchmark tasks 823771f
@arc arc Rename Benchmark::PriorityQueue::Base to ...::Shim
And convert it to a role which `requires` the relevant methods.
@arc arc The Great Renaming
We now have suitable nomenclature for all the sorts of thing that we work
on, so we don't get reduced to calling everything a "benchmark".  And we
also have documentation for the names we've picked.
@arc arc Make program options conform to new nomenclature 16b6b54
@arc arc Check for definedness *before* calling method bda8bb5
@arc arc run_workload(): return number of benchmarks run
Previously returned 1 if any benchmarks were run, or undef otherwise; that
seems fairly useless.
@arc arc Benchmark::PriorityQueue::Shim#run_workload: delete
It wasn't being called anywhere.
@arc arc Create shim instances on demand
Rather than trying to create everything needed up-front.

This also reduces the number of places in the code which know how to map
from a backend name to the name of its corresponding shim class.
@arc arc Make shim instances' attributes immutable cb49066
@arc arc Benchmark::PriorityQueue::Result: new class
Encapsulates the outcome of running a single workload, with enough data to
be able to determine exactly what was run.
@arc arc Benchmark::PriorityQueue: add documentation
Now all functions mentioned in the synopsis have individual documentation.

I've also deleted `make_shim()` from the synopsis, because my current plan
doesn't call for it to be exported.
@arc arc Benchmark::PriorityQueue: export new run_workloads() function
This encapsulates most of the logic needed by frontend programs.
@arc arc bin/bmark_pq: generate output as CSV
This should simplify the task of generating pretty graphs from the results.
@arc arc Delete dead code
The run_workload() function in Benchmark::PriorityQueue no longer has any
non-test callers, so delete it.

With that done, Benchmark::PriorityQueue::Shim#print_benchmark is unused;
deleting that leaves #timed_out unused in turn; and with that gone, it no
longer needs a `timeout` attribute.

Finally, removal of all of those means that we no longer depend on DateTime
or DateTime::Duration.
@arc arc Make timeouts more reliable 3baf8e9
@arc arc bin/bmark_pq: new -o option e2fba1e
@arc arc run_workloads(): accept new `progress` callback 75fc62f
@arc arc bin/bmark_pq: new -v option 068428b
@arc arc bin/bmark_pq: new -r option b9d87ba
@arc arc bin/bmark_pq: refactor for multiple output formats
For now, CSV is the only output format, but we now have a reasonable place
to add more formats (once we also have enough support code to make the
format selectable from the command line).
@arc arc run_workloads(): new `gather` callback c649ff9
@arc arc bin/bmark_pq: new "compare" output format
Uses Benchmark::cmpthese() to generate a table indicating backends' relative
speeds for each task.  There's currently no way to ask for this output
format, though.
@arc arc bin/bmark_pq: new -f option 36ecabf
@arc arc run_workloads(): new `iterations` option 3c250b8
@arc arc bin/bmark_pq: new -i option 5f7387d
@arc arc bin/bmark_pq: abbreviate backend names in "compare" output f4f083f
@arc arc bin/cmp_pq: delete
All of its features (except comparison with multiple ranks) have now been
added to bmark_pq.
@arc arc Benchmark::PriorityQueue: don't export or document make_shim()
It no longer has any callers outside this module itself.

@pozorvlak pozorvlak added a commit that referenced this pull request Jan 8, 2012

@pozorvlak pozorvlak Merge pull request #2 from arc/master
The Great Merging

@pozorvlak pozorvlak merged commit 28dad96 into pozorvlak:master Jan 8, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment