Repository navigation
Cherry pick some dropped Gerrrr commits - #127
StephanDollberg wants to merge 2 commits into
Conversation
Sometimes users want to know about *any* performance change between a PR and a baseline branch, i.e. not just whether there's a regression but also if there's a performance improvement. (cherry picked from commit 116eb5a)
It's useful to get the output of "hunter regressions" in JSON format for post-processing. The default behaviour if the user doesn't pass --output should be the same as before this change. (cherry picked from commit 3400a3c)
|
We are removing the entire regressions command. If you were using it, you are welcome to explain what it is good for. (genuinely) |
|
We use it for feature branch/PR regression testing. Pretty much exactly as described here: https://github.com/Gerrrr/hunter?tab=readme-ov-file#validating-performance-of-a-feature-branch |
|
Hey Stephan! It is awesome to learn that there are more Otava users in the wild. As Henrik mentioned, we've been discussing dropping The TL;DR of those discussions is:
Can you please take a look if there is anything that would be missing for you to switch to Best, |
Yeah we definitely use it a lot 😁 . Both for change point detection via
Ok sounds great. I am happy to migrate if there is feature parity.
I can have a more detailed look tomorrow. From a quick look at the docs I am wondering whether there is similar functionality to:
Thanks for the quick replies. I'll take a deeper look tomorrow. |
|
Partial answer, as I never really used regressions myself, but otoh I'm probably the one with strongest/most confident opinions on these topics, so you can always blame me for bad decisions...
|
Sounds good
100%
Right, that's a fine workaround though having it builtin would be convenient.
Right, we definitley use it for A/B testing. For us the advantage of that is that like this (A: dev/master/main/mergebase, B: PR) we don't have re-run A at all. We just use our existing master data and use otava/change-point-detection to find the correct data to compare to (which the is the point the PR/branch forked from master. Otherwise you might get differences based on changes that are just further ahead on master) We also don't really care about old changepoints for the PR as the latest version is all that matters. But sure we could do some filtering on our side for only the last dataset on the branch/PR. |
Ah that's right. I forgot about this. Is that what --since-commit does/did? I thought it was essentially a synonym for --last??? Can you explain a bit more, how do you provide the data to Otava when you are dealing with a pull request? (I'm not sure this works the same with all the databases...) I just realized in some cases a PR would not be anything special: It is just a history of git commits (child->parent) and it starts (or ends, rather) with the tip of the feature branch. On the other hand if you think of the PR branch as just one or a few commits, then appending them to the end of the main branch is indeed often wrong. It seems I use the commit time to determine the right position of the pr commit in the main branch history. I remember spending some time convincing myself that this is equivalent to looking at merge-base or the chain of parent refrerenses. |
With
Sure, so we use the json provider (not sure whether that's the right word). Then we provide the following data:
Yeah hunter/otava isn't really git aware so it's all just time based. This can cause confusion (it did for us initially) but works fine once you get the timestamps right that you pass to hunter (i.e.: use commit timestamps from the merge commits or similar). |
|
Ok thanks. So this is in the end exactly what I do too, you just thought about it and described it differently. (The Json data source is contributed by me, even :-) ...it seems I didn't contribute any documentation though. Filed #128 The CLI options are evolving for all importers, hopefully the json importer is included by whoever works on those. Anyway, in summary, what I've found works well is that you take your test result history until the merge-base, then add the other branch as a single data point to the end of that time series. (There seems to be both --since and --until options. Personally I deal with this outside of Otava.) And for the purposes of a PR we are only interested in whether this last data point is a change point. (--last 1) So as we evolve the user interface options in Otava, we can keep this in mind. On the mailing list I just proposed a separation of core functionality of the algorithm versus more complex management of exactly things like which commits to include in the first place, or the nature of a PR. Note by the way there is nowadays also an incremental mode, where if you have stored the state of a previous computation, you can cheaply calculate the algorithm only for the tail of the series, because new points are only appended to the end: https://github.com/apache/otava/blob/master/otava/series.py#L325-L407 ...again, this is not exposed in the CLI, and of course using it requires you have available the state from the previous computation of change points. |
|
Thanks, I have now reimplemented our It seems to be working fine. Closing this PR. |
It looks like various commits got dropped when importing the project from Gerrrr/hunter:
This commit cherry picks: