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

Labelling specific commits #191

Open
jbednar opened this issue Jan 9, 2015 · 2 comments
Open

Labelling specific commits #191

jbednar opened this issue Jan 9, 2015 · 2 comments
Labels
enhancement Triaged as an enhancement request

Comments

@jbednar
Copy link

jbednar commented Jan 9, 2015

asv looks great, and we're currently considering using it for performance monitoring for our simulator instead of our own handcrafted and long-broken scripts. From what I can see, the only feature our scripts had that I don't see in asv is the ability to label specific commits with an explanation of what happened with them. We found that when we investigated the commit history, there were a few commits that caused major changes in performance (whether for good or for bad), and once we had completed our investigation we labelled those points on the performance charts, with a key that explains what happened. (E.g. "Added new optimized component XX", "Switched to slower but more general component XX", "Removed potentially unsafe optimization in XX.x", "Rewrote C component in Python using numpy"). Adding these explanations made it clear that we had understood what happened on those specific commits, so that when we came back to look later we didn't start all over again wondering what the big performance drop (or jump) was from. Does such a facility exist in asv? If not, would it be difficult to add? We certainly found it very useful, and appear to have a similar use case to yours...

@mdboom
Copy link
Collaborator

mdboom commented Jan 9, 2015

Such a feature doesn't currently exist, but it seems like it would be very useful. It does support showing any git tags in the graph -- but I understand that's not exactly what you want, but that feature could be extended to support what you want fairly easily, I would think.

You could probably experiment with it from outside in fact. When you do an asv publish to generate the html/json, in html/index.json there should be a tags entry, like:

"tags": {
        "v0.1": 1340086193000, 
        "v0.2": 1361312808000, 
        "v0.2.1": 1365018342000, 
        "v0.2.2": 1369169818000, 
        "v0.2.3": 1369943695000, 
        "v0.2.4": 1374690436000, 
        "v0.2.5": 1382718598000, 
        "v0.2b1": 1356381732000, 
        "v0.2b2": 1359508111000, 
        "v0.2rc1": 1360776151000, 
        "v0.3": 1384996150000, 
        "v0.3.1": 1393955310000, 
        "v0.3.2": 1399997164000, 
        "v0.3b1": 1383542385000, 
        "v0.3rc1": 1384388274000, 
        "v0.4": 1405527129000, 
        "v0.4.1": 1407512129000, 
        "v0.4.2": 1411505522000, 
        "v0.4rc1": 1404233912000, 
        "v0.4rc2": 1405176393000
    }

The first is a string, the second is a javascript time stamp of an associated commit. You could add comments in the string part as long as you know the date time stamp, and they should appear in the graph.

Of course, all that would need to be wrapped up in something more convenient that took pairs on commits and comments from the asv.conf.json file, and passed them along and merged them into this tag list by asv publish.

One wrinkle is that you may not want these comments to appear for all benchmarks. Certain refactoring changes may only impact certain parts of your library/application. That would require a mapping from benchmark names to these annotations as well, and then some more smarts on the javascript side about when to show certain annotations. That's a lot more work than the approach I describe above, but not impossible by any means.

@jbednar
Copy link
Author

jbednar commented Jan 9, 2015

Ok, thanks. That sounds promising. Yes, the git tag or even the git commit message shows some information, but not necessarily what will help us understand performance changes retroactively, and so your proposal of generating such a mapping specifically for annotations seems appropriate. I too thought of the issue of multiple benchmarks for changes that affect only one of them, but that level of specificity seems like a feature that could be left safely far in the future. The worst that would happen with a simple benchmark-global annotation is that we'd get an irrelevant annotation on some point of the graph where the performance didn't actually change for that particular benchmark. I think it would be easy enough to ignore such annotations (since no one will be closely investigating parts of a benchmark graph where nothing interesting is happening!), and so probably not worth the effort to refactor the tags support at both ends as you describe.

@pv pv mentioned this issue Mar 6, 2015
11 tasks
@pv pv added the enhancement Triaged as an enhancement request label Jun 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Triaged as an enhancement request
Projects
None yet
Development

No branches or pull requests

3 participants