Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add commit description and ordering to output #139

Merged
merged 2 commits into from Jun 27, 2019

Conversation

schneems
Copy link
Member

@schneems schneems commented Jun 27, 2019

There were some problems to the old formatting of the library comparison
output. The x faster and % faster were confusing because it wasn't clear
how they were derived. Also it was not clear which commit SHA belonged
to which values.

In this updated value we give the formulas for the values that are being
shown as well. The latest commit is always first and the value will show
either SLOWER or FASTER based on actual values. Finally we're including
the git first line description to make it clearer which commit is which.

New output looks like this:

❤️ ❤️ ❤️  (Statistically Significant) ❤️ ❤️ ❤️

[7b4d80cb37] "1.8x Faster Partial Caching - Faster Cache Keys" - (10.9711965 seconds)
  FASTER by:
    1.0870x [older/newer]
    8.0026% [(older - newer) / older * 100]
[13d6aa3a7b] "Merge pull request #36284 from kamipo/fix_eager_loading_with_string_joins" - (11.9255485 seconds)

P-value: 4.635595463712749e-05
Is significant? (P-value < 0.05): true

There were some problems to the old formatting of the library comparison
output. The x faster and % faster were confusing because it wasn't clear
how they were derived. Also it was not clear which commit SHA belonged
to which values.

In this updated value we give the formulas for the values that are being
shown as well. The latest commit is always first and the value will show
either SLOWER or FASTER based on actual values. Finally we're including
the git first line description to make it clearer which commit is which.

New output looks like this:

```
❤️ ❤️ ❤️  (Statistically Significant) ❤️ ❤️ ❤️

[7b4d80cb37] "1.8x Faster Partial Caching - Faster Cache Keys" - (10.9711965 seconds)
  FASTER by:
    1.0870x [older/newer]
    8.0026% [(older - newer) / older * 100]
[13d6aa3a7b] "Merge pull request #36284 from kamipo/fix_eager_loading_with_string_joins" - (11.9255485 seconds)

P-value: 4.635595463712749e-05
Is significant? (P-value < 0.05): true
```
@schneems schneems force-pushed the schneems/more-info-rails-bench branch from f0d44b7 to fced9e2 Compare June 27, 2019 22:02
If you use a branch name with the method that is given i.e. "master" it
can mess things up since "master" is the commit SHA you specify in the
Gemfile.lock and not the actual master value (though this works
correctly if you're specifying path: in your Gemfile).

We can use SHA instead of BRANCHES for the input. Also just plain
"library" instead of "library_branches"
@schneems schneems force-pushed the schneems/more-info-rails-bench branch from 3bcf845 to b3f5988 Compare June 27, 2019 22:22
@schneems schneems merged commit b70e8a3 into master Jun 27, 2019
if significant?
io.puts "❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️"
io.puts "❤️ ❤️ ❤️ (Statistically Significant) ❤️ ❤️ ❤️"
Copy link
Contributor

Choose a reason for hiding this comment

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

So much love 💌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants