allow talos to run with xperf and upload results to autolog

RESOLVED FIXED

Status

Testing
Talos
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jmaher, Assigned: jmaher)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 years ago
Created attachment 553895 [details] [diff] [review]
log results to autolog from xperf talos (1.0)

now that we have xperf integrated into talos, we need to upload the results to autolog.
Attachment #553895 - Flags: review?(jgriffin)
Attachment #553895 - Flags: feedback?(anodelman)
Comment on attachment 553895 [details] [diff] [review]
log results to autolog from xperf talos (1.0)

Review of attachment 553895 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good!  I only reviewed the autolog parts of this code, since I'm not really familiar with what else is happening.

::: xtalos/etlparser.py
@@ +57,5 @@
>  
> +class XPerfAutoLog(object):
> +  testGroup = None
> +
> +  def __init__(self, filename = None):

Safer to put 'testgroup = None' inside __init__(), in case two XPerfAutoLog objects exist at once at some point in the future.

@@ +98,5 @@
> +      
> +    self.testGroup.add_perf_data(
> +      test = self.yaml_config['testname'],
> +      type = 'diskIO',
> +      name = filename,

I notice that the filename is usually a full filename, something like "\Device\HarddiskVolume3\Users\cltbld\AppData\Local\Temp\tmp3ejelh\profile\cookies.sqlite".  The perf dashboard won't be able to graph data for this very well, since different runs will have different paths.  I think it might be better just to pass the file's name, e.g.,

name = filename[filename.rfind('\\') + 1:],

so you can track performance across test runs.

(And yes, rfind exists in python 2.4...I checked!)

@@ +281,5 @@
>           files[row]['DiskReadBytes'],
>           files[row]['DiskWriteCount'],
>           files[row]['DiskWriteBytes'])
> +         
> +    if alog:

I don't see where alog is defined in the case where not options.configFile; if this happens, this line will generate:

NameError: name 'alog' is not defined
Attachment #553895 - Flags: review?(jgriffin) → review+
(Assignee)

Comment 2

6 years ago
Created attachment 554052 [details] [diff] [review]
log results to autolog from xperf talos (1.1)

updated patch with :jgriffin's comments, tested on tools-staging and it is all green.
Assignee: nobody → jmaher
Attachment #553895 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #553895 - Flags: feedback?(anodelman)
Attachment #554052 - Flags: review+
Attachment #554052 - Flags: feedback?(anodelman)
Comment on attachment 554052 [details] [diff] [review]
log results to autolog from xperf talos (1.1)

I'm still wary that we are doing:

+      os.system('python xtalos\\stop_xperf.py -x %s' % (self.xperf_path))

As that looks distinctly blocking and could get a machine stuck.
Attachment #554052 - Flags: feedback?(anodelman) → feedback+
(Assignee)

Comment 4

6 years ago
http://hg.mozilla.org/build/talos/rev/ff3b05d67fcb
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

6 years ago
Depends on: 680522
You need to log in before you can comment on or make changes to this bug.