Various changes to perf code #3037
Conversation
|
Skipping CI for Draft Pull Request. |
istio-testing
added
do-not-merge/work-in-progress
Stevenjin8
force-pushed
the
perf/small-fixes
branch
from
092512b
to
3c7086c
Compare
Stevenjin8
changed the title
Stevenjin8
force-pushed
the
perf/small-fixes
branch
5 times, most recently
from
48a81ab
to
a65a20b
Compare
Stevenjin8
force-pushed
the
perf/small-fixes
branch
from
a65a20b
to
643546c
Compare
| @@ -174,15 +179,20 @@ def get_parser(): | |||
| parser.add_argument( | |||
| "--query_str", | |||
| help="Specify the qps or conn query_str that will be used to query your y-axis data based on the CSV file." | |||
| "For example: conn_query_str=ActualQPS==1000, qps_query_str=NumThreads==16." | |||
| "For example: conn_query_str=ActualQPS==1000, qps_query_str=NumThreads==16.", | |||
There was a problem hiding this comment.
not sure what that example is but if ActualQPS is from fortio json, it's very unlikely to be a nice exact round number - if you ask for 1000 you're likely to get 999.97 or some such
There was a problem hiding this comment.
I think my formatter might have caught this. Having the hard equals isn't great, I agree. But it gets rounded to an int, so if you run it for long enough, the actual qps ends up being the desired qps (most of the time).
| else: | ||
| if not data[metric_name].head().empty: | ||
| if metric_name.startswith('cpu') or metric_name.startswith('mem'): | ||
| y_series_data.append(data[metric_name].head(1).values[0]) | ||
| else: | ||
| y_series_data.append(data[metric_name].head(1).values[0] / data["ActualQPS"].head(1).values[0]) | ||
| y_series_data.append(data[metric_name].head(1).values[0] / 1000) |
There was a problem hiding this comment.
not sure why it was using data["ActualQPS"] before but maybe comment on the change?
(also fortio data, in the json files, is in seconds not ms (even if the UI shows in ms for human consumption) so not sure what the 1000 is for either?
There was a problem hiding this comment.
After some digging I think this is correct (but a comment would help). Fortio reports data in seconds, but the script that pull data from the fortio client converts it to microseconds
tools/perf/benchmark/runner/fortio.py
Lines 60 to 64 in 2030a1d
so I have to divide by 1000 to get it back to milliseconds
istio-testing
removed
the
do-not-merge/work-in-progress
|
@Stevenjin8: The following tests failed, say
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
No description provided.