-
Notifications
You must be signed in to change notification settings - Fork 97
Automated Travis benchmark [ESD-1158] #667
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
Conversation
a2b76e5 to
a78c60f
Compare
2a15659 to
3a911a7
Compare
|
|
fd3039c to
54f03c3
Compare
|
Working locally. Some tune up TODO for docker cooperation. |
36371b9 to
1e9ffeb
Compare
Will need to revisit this in other usages, this is probably more correct behavior but we'll need to adjust users of this iterator to expect the new behavior.
1e9ffeb to
f24f1f9
Compare
|
Based on the runs this far, the performance ratio threshold could be set somewhere around 1.8. |
| @@ -0,0 +1,49 @@ | |||
| import cffi | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prior to merging this we need to figure out how to get this module to generate itself automatically (for development), then build a compiled version for a PyPI wheel (this is optional, since it might be worthy of a whole separate PR to figure out to build and distribute this).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about a first phase remedy:
from sbp.jit import parse_float # effectively runs the script building the module `parse_float_c`
from sbp.jit import parse_float_c
Also note the additions to tox.ini so another possibility would be to add the requirement to run make python before usage but that would probably go unnoticed in quite a lot of cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pmiettinen that might work, as long as the import succeeds, hopefully this doesn't require the user to have write access to wherever sbp.jit.parse_float is installed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty sure the write access is required but I don't think this it matters at this phase as this solution is aimed only for development and the files shall reside under the repository? Do this now and later a better solution in a PR adding the sbp.jit package into the release PyPi package?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
This is done on top of #656
And paired with swift-nav/piksi_tools#985