GNOME Bugzilla – Bug 732349
Fixes for perf framework
Last modified: 2014-06-29 22:32:00 UTC
Here are some miscellaneous fixes and cleanups for the perf framework; I'll file another bug in a second that depends on these fixes and adds a new performance script used for hardware performance testing.
Created attachment 279409 [details] [review] Scripting: exit if the perf script throws an error during collection If an exception occurs while collecting parameters, exit rather than just leaving the shell running in the main loop.
Created attachment 279410 [details] [review] perf: fix bug in code to print out metrics We don't normally hit the code in scripting.js to print metrics because shell-perf-tool bypasses it, but there was a left-over in the code that no longer works. Also add in the units to the output.
Created attachment 279411 [details] [review] Scripting.createTestWindow(): take params Take a parameter object rather than a long parameter list containing multiple booleans.
Created attachment 279412 [details] [review] Scripting: catch errors from shell-perf-helper When calling remote D-Bus methods, handle exceptions, log them and exit, rather than just proceeding as if nothing happened.
Review of attachment 279409 [details] [review]: OK.
Review of attachment 279410 [details] [review]: OK.
Review of attachment 279411 [details] [review]: OK.
Review of attachment 279412 [details] [review]: OK. ::: js/ui/scripting.js @@ +103,3 @@ + let errcb; + + let args = Array.slice(arguments, 2); Hm, I didn't expect that to work. Turns out it's an undocumented part of Mozilla JS. I'd use the more traditional [].slice.call(arguments, 2);
(In reply to comment #8) > Review of attachment 279412 [details] [review]: > > OK. > > ::: js/ui/scripting.js > @@ +103,3 @@ > + let errcb; > + > + let args = Array.slice(arguments, 2); > > Hm, I didn't expect that to work. Turns out it's an undocumented part of > Mozilla JS. I'd use the more traditional [].slice.call(arguments, 2); It's documented in MDN (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array - see "Array Generic Methods") But yeah, somehow I had it in my head that it was scheduled for Ecmascript 6 and it's not. Even if JS engines optimize out the allocation in [].slice.call(), it still looks pretty ugly to me, Array.prototype.slice.call(arguments, 2) seems cleaner. Or how do you feel about using something that *is* slated for Ecmascript 6 function _callRemote(obj, method, ...args) { } ?
(In reply to comment #9) > Or how do you feel about using something that *is* slated for Ecmascript 6 > > function _callRemote(obj, method, ...args) { > } > > ? As long as it works in gjs, sure.
Pushing (with change to use ...args) Attachment 279409 [details] pushed as 910c95f - Scripting: exit if the perf script throws an error during collection Attachment 279410 [details] pushed as e375e1a - perf: fix bug in code to print out metrics Attachment 279411 [details] pushed as f285f2c - Scripting.createTestWindow(): take params Attachment 279412 [details] pushed as 2fbd8f0 - Scripting: catch errors from shell-perf-helper