xpcshell test "unit/test_nsIProcess.js" is slow on Windows

RESOLVED FIXED

Status

()

Core
XPCOM
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: wlach, Assigned: wlach)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [buildfaster:p1])

Attachments

(1 attachment)

As part of the GoFaster project (to reduce build times), I've been profiling the various xpcshell tests. The "unit/test_nsIProcess.js" is about 13 seconds slower on our windows test slaves than on my Linux machine, a difference which I've pinned down almost entirely to this bit of code:

  for (var i = 0; i < 1000; i++) {
    var process = Components.classes["@mozilla.org/process/util;1"]
                          .createInstance(Components.interfaces.nsIProcess);
    process.init(file);

    process.run(false, [], 0);
    ...

I gather we are doing a mini stress test here, but how important is it that we do 1000 iterations of this? Would 100 suffice?
Hey Mike, from what I can gather from 'hg blame', you're the author of the code I'm talking about above. Do you have any thoughts on it?
From my experimentation back when i wrote the test initially, the bug it's trying to find would only trigger once in a while, and 1000 got it reliably, while 100 didn't. I haven't tried other values in between, and I haven't bothered because 1000 was fast enough... on Linux. I forgot that Windows was slow to fork processes...

Now, looking at the code that the test verifies (bug 543441 ; turns out the bug number in the corresponding commit was wrong :( ), it actually doesn't apply to windows, so we could actually skip the test on windows.
Whiteboard: [buildfaster:p1]
Created attachment 551891 [details] [diff] [review]
split out the process stress test into a seperate file and skip it on win7
Assignee: nobody → wlachance
Comment on attachment 551891 [details] [diff] [review]
split out the process stress test into a seperate file and skip it on win7

Hi Benjamin,

glandium suggested I assign this to you to review, as you're the module owner.
Attachment #551891 - Flags: review?(benjamin)

Updated

6 years ago
Attachment #551891 - Flags: review?(benjamin) → review+
I should add that I tested this commit by running the xpcshell tests on try against all platforms. Still passes!
Landed as http://hg.mozilla.org/mozilla-central/rev/bd5215405aa9
Awesome, thanks!
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.