679865 - allow talos to run with xperf and upload results to autolog

Status: RESOLVED FIXED

(Testing :: Talos, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: 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.

Comment 1

[Jonathan Griffin (:jgriffin)]

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

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: log results to autolog from xperf talos (1.1)

updated patch with :jgriffin's comments, tested on tools-staging and it is all green.

Comment 3

[alice nodelman [:alice] [:anode]]

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.

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

http://hg.mozilla.org/build/talos/rev/ff3b05d67fcb