Closed
Bug 821242
Opened 12 years ago
Closed 11 years ago
Need more intelligent sort for double digit machine names (eg 'R10')
Categories
(Tree Management Graveyard :: TBPL, defect)
Tree Management Graveyard
TBPL
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: emorley, Assigned: RyanVM)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
emorley
:
review+
|
Details | Diff | Splinter Review |
B2G Arm emulator now runs reftest in 10 chunks. Resultant order is:
R1 R10 R2 R3 R4....
Reporter | ||
Comment 1•11 years ago
|
||
Relevant code is at:
https://hg.mozilla.org/webtools/tbpl/file/0a3def9520dd/js/Data.js#l421
421 if (Config.groups.indexOf(group) != -1) {
422 // Sort the jobs by type (eg reftest/crashtest), then by chunk number.
423 // The space is so "Mochitest [0-9]" sorts before "Mochitest {Browser Chrome,Other}"
424 var aNum = result.machine.type + ' ' + result.machine.jobPartNumber();
425 comparator = function (b) {
426 var bNum = b.machine.type + ' ' + b.machine.jobPartNumber();
427 if (aNum == bNum)
428 return compareStateTime(b);
429
430 return aNum > bNum ? 1 : -1;
431 }
432 }
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → ryanvm
Assignee | ||
Comment 2•11 years ago
|
||
Attaching this as a checkpoint. It works :)
Does not include the optimization work we discussed on IRC, though.
Assignee | ||
Comment 3•11 years ago
|
||
This avoids running the jobPartNumber regex on the second job in the common case where the job types are different.
Attachment #8379101 -
Attachment is obsolete: true
Attachment #8379122 -
Flags: review?(emorley)
Reporter | ||
Comment 4•11 years ago
|
||
Comment on attachment 8379122 [details] [diff] [review]
proper chunk handling avoiding unnecessary work in the common case
Review of attachment 8379122 [details] [diff] [review]:
-----------------------------------------------------------------
r+ with the changes below :-)
::: js/Data.js
@@ +420,5 @@
> var comparator = compareStateTime;
> if (Config.groups.indexOf(group) != -1) {
> // Sort the jobs by type (eg reftest/crashtest), then by chunk number.
> + var aNum = parseInt(result.machine.jobPartNumber());
> + var aType = result.machine.type;
Per IRC, can we swap these two variable declarations to be semantically less to more specific? :-)
@@ +427,5 @@
> + if (aType != bType)
> + return aType > bType ? 1 : -1;
> +
> + var bNum = parseInt(b.machine.jobPartNumber());
> +
With the next comment implemented, think we can ditch the newline here :-)
@@ +432,4 @@
> if (aNum == bNum)
> return compareStateTime(b);
>
> return aNum > bNum ? 1 : -1;
Could we swap these last two return statements to make the comparisons semantically in order of least to most specific?
ie: [compare type] -> [compare number] -> [last resort: compareStateTime]
Attachment #8379122 -
Flags: review?(emorley) → review+
Assignee | ||
Comment 5•11 years ago
|
||
(In reply to Ed Morley [:edmorley UTC+0] from comment #4)
> Per IRC, can we swap these two variable declarations to be semantically less
> to more specific? :-)
Whoops, I made that change locally and forgot to update the attached patch apparently :)
> With the next comment implemented, think we can ditch the newline here :-)
Done.
> Could we swap these last two return statements to make the comparisons
> semantically in order of least to most specific?
> ie: [compare type] -> [compare number] -> [last resort: compareStateTime]
Done.
https://hg.mozilla.org/webtools/tbpl/rev/aa8f3a0e2308
Assignee | ||
Comment 6•11 years ago
|
||
In production :)
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
Product: Webtools → Tree Management
Updated•10 years ago
|
Product: Tree Management → Tree Management Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•