--spsProfile doesn't work from try syntax

RESOLVED FIXED in Firefox 49

Status

RESOLVED FIXED
3 years ago
a year ago

People

(Reporter: jmaher, Unassigned)

Tracking

unspecified
mozilla49
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox49 fixed)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
on march 15th, mconley was successful in pushing to try and getting sps profiles:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=034a6b40d711

I tried a day later and it failed:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=feb9b7b467b2

I am not sure why this failed, but we should make this more foolproof.
(Reporter)

Comment 1

3 years ago
here is the code that does the parsing of the try syntax to enable spsProfiling:
https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/mozilla/testing/talos.py#167
What tests are you concerned with? And are we mostly concerned with e10s or non-e10s here?
(Reporter)

Comment 4

2 years ago
in this case regular non-e10s
(Reporter)

Comment 5

2 years ago
ok, if the comment is multi line, the parsing fails, which my push is, and bholley's push, but mconley has a single line comment.
(Reporter)

Updated

2 years ago
Flags: needinfo?(jmaher)
(Reporter)

Comment 6

2 years ago
Created attachment 8749649 [details]
MozReview Request: Bug 1257570 - --spsProfile doesn't work from try syntax. r?wlach

Review commit: https://reviewboard.mozilla.org/r/51085/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/51085/
Attachment #8749649 - Flags: review?(wlachance)
Attachment #8749649 - Flags: review?(wlachance)
Comment on attachment 8749649 [details]
MozReview Request: Bug 1257570 - --spsProfile doesn't work from try syntax. r?wlach

https://reviewboard.mozilla.org/r/51085/#review47741

Awesome! There are some minor revisions I'd like to see before this goes in.

::: testing/mozharness/mozharness/mozilla/testing/talos.py:177
(Diff revision 1)
>              # now let's see if we added spsProfile specs in the commit message
>              try:
>                  junk, junk, opts = self.buildbot_config['sourcestamp']['changes'][-1]['comments'].partition('mozharness:')
> +
> +                # we need to guard against multi line comments
> +                opts = opts.split('\n')[0]

You should put this lower, like line 183, since it's not strictly part of the try...except clause.

I'd also make the comment something like:

```py
# In the case of a multi-line commit message, only examine the first line for mozharness options
```

(you might need to split that across 2 lines)
(Reporter)

Comment 8

2 years ago
Comment on attachment 8749649 [details]
MozReview Request: Bug 1257570 - --spsProfile doesn't work from try syntax. r?wlach

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/51085/diff/1-2/
Attachment #8749649 - Flags: review?(wlachance)
Comment on attachment 8749649 [details]
MozReview Request: Bug 1257570 - --spsProfile doesn't work from try syntax. r?wlach

https://reviewboard.mozilla.org/r/51085/#review47781
Attachment #8749649 - Flags: review?(wlachance) → review+

Comment 11

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/47bc46e92a73
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox49: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
This seems to be broken again. The tests run, but I don't get any profile data.
Might be related to bug 1331807? Probably worth filing a new bug.
I don't think so. From the logs, it doesn't look like the sps profile is even being recorded. On Linux, anyway. On Windows, the tests fail to run, but it looks like it is trying to record profile data (which I think is bug 1295644).

Updated

2 years ago
Blocks: 1333167
Blocks: 1354964
You need to log in before you can comment on or make changes to this bug.