Remove mozprocess.pid, mozprocess.wpk

RESOLVED FIXED in Firefox 44

Status

Testing
Mozbase
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: wlach, Assigned: parkouss)

Tracking

unspecified
mozilla44
x86_64
Linux
Points:
---

Firefox Tracking Flags

(firefox44 fixed)

Details

MozReview Requests

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

Attachments

(4 attachments)

These functions are known to be broken (see bug 1138759). For code in mozilla-central, we can just use psutil which is included there (nothing there uses mozprocess.pid right now though). The only other consumer of this code is talos. I'd just copy the relevant code into talos directly. If/when we move talos into mozilla-central, we can migrate it to use psutil as well.
Created attachment 8573565 [details] [diff] [review]
Remove mozprocess.pid, mozprocess.wpk

This removes these functions from mozprocess.
Assignee: nobody → wlachance
Attachment #8573565 - Flags: review?(ahalberstadt)
Created attachment 8573566 [details] [diff] [review]
Make talos use psutil instead mozprocess.pid

obv this needs a try run, doing one
Attachment #8573566 - Flags: review?(jmaher)
Try run for mozbase unit tests and talos: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e91dfac29c85
Attachment #8573565 - Flags: review?(ahalberstadt)
Attachment #8573566 - Flags: review?(jmaher)
Unfortunately this approach doesn't work with Windows talos, as it requires compiling some parts of psutil which aren't supported in that environment. I assume this problem has somehow been solved for our other tests (probably by precompiling psutil and putting it in tests.zip), if we ever moved talos in-tree (bug 787200) we would be ok pushing this code. As it is, solving this in alternate ways probably isn't worth the effort. Let's just leave things as they are for now and maybe revisit in the future.
Not working on this at the moment.
Assignee: wlachance → nobody
See Also: → bug 787200
ok, psutil is working now, this might be worth revisiting.
(Assignee)

Comment 7

2 years ago
Created attachment 8670445 [details]
MozReview Request: Bug 1139487 - Remove mozprocess.pid usage from talos. r=jmaher

Bug 1139487 - Remove mozprocess.pid usage from talos. r=jmaher
Attachment #8670445 - Flags: review?(jmaher)
(Assignee)

Comment 8

2 years ago
Created attachment 8670446 [details]
MozReview Request: Bug 1139487 - Remove mozprocess.pid, mozprocess.wpk. r=jmaher

Bug 1139487 - Remove mozprocess.pid, mozprocess.wpk. r=jmaher
Attachment #8670446 - Flags: review?(jmaher)
(Assignee)

Comment 9

2 years ago
So, let's see how that goes - I ran a push try on the first patch only,

https://treeherder.mozilla.org/#/jobs?repo=try&revision=cb30fddb5d69

We should ensure that numbers are still consistent.

I tried with dxr to find all mozprocess.pid, mozprocess.wpk usage - seems to me like talos was alone. But maybe a full push to try would be good here to be sure ?
(Assignee)

Updated

2 years ago
Assignee: nobody → j.parkouss
Status: NEW → ASSIGNED
Comment on attachment 8670446 [details]
MozReview Request: Bug 1139487 - Remove mozprocess.pid, mozprocess.wpk. r=jmaher

https://reviewboard.mozilla.org/r/21367/#review19253

this is nice!
Attachment #8670446 - Flags: review?(jmaher) → review+
Attachment #8670445 - Flags: review?(jmaher) → review+
Comment on attachment 8670445 [details]
MozReview Request: Bug 1139487 - Remove mozprocess.pid usage from talos. r=jmaher

https://reviewboard.mozilla.org/r/21365/#review19255

this is more of a rubber stamp, this change confuses me, not the change, just the code it is in!
(Assignee)

Comment 12

2 years ago
So, we should be good, looking at talos results there are no obvious changes:

https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-inbound&originalRevision=d01dd42e654b&newProject=try&newRevision=cb30fddb5d69

So I should still run a try on the two patches just to be sure unit tests are good for each platform.
(Assignee)

Comment 13

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=51bc43ca702d
(Assignee)

Comment 14

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/9d52d889c77bd25e2ccefcd609570e0b371863d9
Bug 1139487 - Remove mozprocess.pid usage from talos. r=jmaher

https://hg.mozilla.org/integration/mozilla-inbound/rev/c57207f4c801c1af8ded94db984b3fb802e434fc
Bug 1139487 - Remove mozprocess.pid, mozprocess.wpk. r=jmaher
https://hg.mozilla.org/mozilla-central/rev/9d52d889c77b
https://hg.mozilla.org/mozilla-central/rev/c57207f4c801
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.