M9: Build performance test report view #12
Conversation
ninetteadhikari
added
only for review
| { | ||
| start: 0, | ||
| end: 100 | ||
| } |
There was a problem hiding this comment.
It seems like the second one sets the default boundary of the zoom. But I am removing the first one because it seems like its not doing anything 🤷
| // Change chart size with browser resize | ||
| window.addEventListener('resize', function() { | ||
| measurement_chart.resize(); | ||
| }); | ||
| measurement_chart.setOption(option); |
There was a problem hiding this comment.
this one is nice since it makes the charts responsive to screen size. Otherwise user will have to reload the page to see the chart, like this:
There was a problem hiding this comment.
Mh, they don't have this in the old version. Do you know if the google charts resize?
| position: function (pt) { | ||
| return [pt[0], '10%']; | ||
| }, |
There was a problem hiding this comment.
this just made the tooltip position fixed closer to the cursor. Its just aesthetics not necessary, so I can remove it.
| google.charts.setOnLoadCallback(drawChart_{{ chart_elem_id }}); | ||
| function drawChart_{{ chart_elem_id }}() { | ||
| var data = new google.visualization.DataTable(); | ||
| <script type="module"> |
There was a problem hiding this comment.
question why does it need to be a module?
| pointSize: 5, | ||
| chartArea: { left: 80, right: 15 }, | ||
| }; | ||
| const convertToMinute = (time) => { |
There was a problem hiding this comment.
Can you name it a bit more specific?
Like convertMSToMinute or whatever the time format here is? :)
| new Date(time * 1000).getTime(), // The Date object takes values in milliseconds rather than seconds. So to use a Unix timestamp we have to multiply it by 1000. | ||
| Array.isArray(value) ? convertToMinute(value) : value // Assuming the array values are duration in the format [hours, minutes, seconds, milliseconds] |
There was a problem hiding this comment.
Please move the comments above the code. not next to it
| new Date(time * 1000).getTime(), // The Date object takes values in milliseconds rather than seconds. So to use a Unix timestamp we have to multiply it by 1000. | ||
| Array.isArray(value) ? convertToMinute(value) : value // Assuming the array values are duration in the format [hours, minutes, seconds, milliseconds] |
There was a problem hiding this comment.
I'm a bit confused here. it is time or value. and what format has the time or value?
| [{{ sample.commit_num }}, {{ sample.mean.gv_value() }}], | ||
| {% endfor %} | ||
| ]); | ||
| // Convert raw data to the format: [time, value] |
There was a problem hiding this comment.
this comment can be dropped. looks outdated?
scripts/lib/build_perf/report.py
Outdated
| @@ -319,6 +319,7 @@ def measurement_stats(meas, prefix=''): | |||
| stats['quantity'] = val_cls.quantity | |||
| stats[prefix + 'sample_cnt'] = len(values) | |||
|
|
|||
| start_time = time # Add start time for both type sysres and disk usage | |||
There was a problem hiding this comment.
Please no comments next to the code :)
ninetteadhikari
force-pushed
the
m9-performance-test-report-view
branch
from
95fbbf4
to
4a25aa0
Compare
ninetteadhikari
force-pushed
the
m9-performance-test-report-view
branch
from
0759af1
to
0ff5f57
Compare
ninetteadhikari
added
only for review
ninetteadhikari
force-pushed
the
m9-performance-test-report-view
branch
4 times, most recently
from
32ab08d
to
5c0c415
Compare
Signed-off-by: Ninette Adhikari <[email protected]>
ninetteadhikari
force-pushed
the
m9-performance-test-report-view
branch
from
5c0c415
to
1d5e7db
Compare
This PR is for "Milestone 9: Build performance test report view" as stated in the Scope of Work. It addresses the following:
start_timeso that time can be displayed instead of commit numbers on the chart. It also updates default commit history length to 300.