Open Bug 909577 Opened 8 years ago Updated 8 years ago

API for reading process info

Categories

(Toolkit :: General, defect)

defect
Not set
normal

Tracking

()

ASSIGNED

People

(Reporter: gps, Assigned: mihneadb)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 7 obsolete files)

:mihneadb is working on instrumenting our test harnesses to record process resource usage (CPU, I/O, memory, etc). After much mucking around, it's becoming clearer that obtaining resource usage from within the XPCOM process is simpler than obtaining it from an external process (such as the Python test runner).

I'm proposing adding functionality to Gecko to quickly and easily access the current process's basic resource info. Initially, I'm thinking this will be a JSM. Eventually, we can convert it to an XPCOM component and/or rewrite it in C++.

https://hg.mozilla.org/mozilla-central/file/14b1e8c2957e/toolkit/components/telemetry/TelemetryPing.js#l104 is some existing code that does something similar.
Assignee: nobody → mihneadb
Attached patch API for reading process info (obsolete) — Splinter Review
First iteration
Blocks: 907455
Attachment #795787 - Attachment is obsolete: true
I am suspicious of the idea of reading a file (even one from /proc) on the main thread. Can anyone tell with absolute certainty that this will not cause main thread I/O?
Flags: needinfo?(mihneadb)
When you'd use the numbers to compare two processes against each other you will basically have the same thing in both, so I'd say it is negligible.

Or maybe I didn't get the problem you are referring to?
Flags: needinfo?(mihneadb)
(Just for the record, this module's purpose is profiling resource usage. If you are worried about loss of smoothness, I'd say this is not an issue for this use case.)
(In reply to David Rajchenbach Teller [:Yoric] from comment #3)
> I am suspicious of the idea of reading a file (even one from /proc) on the
> main thread. Can anyone tell with absolute certainty that this will not
> cause main thread I/O?

We should have a conversation about this!

I advised Mihnea to use synchronous/main thread I/O for reading from /proc because I don't believe access to the /proc/self "files" here will delay the main thread because /proc is a virtual filesystem that effectively provides access to kernel data structures. Therefore, I view reading content from *certain* /proc "files" as the equivalent of calling an API like getrusage(), something we wouldn't need to offload to a child thread.

I'm assuming that under the hood nsIFile is just a wrapper around the open/read/close system calls and doesn't do anything complicated like marshal requests to different threads (like OS.File).

Furthermore, I think that obtaining resource usage should have as little probe overhead as possible. OS.File with it's model of moving requests to a different thread - a good model for most scenarios - just seems overkill for this.

Also, we don't *currently* intend for this API to be used in shipping code - just by test harnesses. However, once the cat is out of the bag and this module ships, all bets are off.

All that being said, I would like sign-off from someone on the performance team that synchronous I/O for reading select files from /proc is acceptable. I hope I've presented sufficient justification for it not being an issue.
Comment on attachment 795816 [details] [diff] [review]
API for reading process info

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

Nice!

I think you need to work on the API. Specifically, we don't want callers to be concerned with the platform type - we should expose a high-level "getIOCounters" etc that "just works." This likely means normalizing the returned data to a common format similar to how psutil does it.

We'll also need some form of error handling. On Linux, I'm pretty sure the format of the /proc/self/* structures you are reading are different in older kernel versions. We should raise a proper exception type (not file not found) if we have errors reading these.

::: toolkit/modules/ProcessInfo.jsm
@@ +13,5 @@
> +
> +
> +function getFileFromPath(path) {
> +  let file = Cc["@mozilla.org/file/local;1"]
> +             .createInstance(Ci.nsILocalFile);

Nit: Align .createInstance under the [ (2 space indent):

Cc["@mozilla.org/file/local;1"]
  .createInstance(Ci.nsILocalFile)
  .foo(xxx);

@@ +22,5 @@
> +function readFileAsLines(path) {
> +  let file = getFileFromPath(path);
> +  // open an input stream from file
> +  let istream = Cc["@mozilla.org/network/file-input-stream;1"].
> +    createInstance(Ci.nsIFileInputStream);

Nit: Put the . on the 2nd line and align under the [ from Cc[ (it's how we do things - don't ask).

@@ +38,5 @@
> +
> +  return lines;
> +}
> +
> +function Linux__getIOCounters() {

We typically don't use underscores in function names. I've never seen double underscore used.

@@ +45,5 @@
> +  let data = {};
> +  lines.map(function(line) {
> +    let [key, value] = line.split(":");
> +    data[key] = value | 0;
> +  });

Since we are doing this from performance-sensitive code, might want to avoid the .map() and just iterate over the lines.

@@ +69,5 @@
> +  let startPos = line.indexOf(")") + 2;
> +  let values = line.substring(startPos).split(" ");
> +  return {
> +    utime: values[11] | 0,
> +    stime: values[12] | 0

cutime and cstime would also be interesting so we can look at low-level cycle counts over time.
Well, if this code isn't meant to be shipped in production, I guess that everything is permitted.

However, if it does perform main thread I/O (even something as simple as open/read/close), you should remember that every operation has an unbounded and unpredictable duration. On a busy system, this can be orders of magnitude worse than sending a message to a dedicated thread (seconds vs. milliseconds).

If you consider that the overhead of OS.File is unacceptable, you should write the code in C++ and communicate with Dispatch.
(In reply to David Rajchenbach Teller [:Yoric] from comment #8)
> Well, if this code isn't meant to be shipped in production, I guess that
> everything is permitted.

Well, it needs to be shipped (unless we make it a testing-only module). Perhaps we should add a prominent comment and/or a console.warning when the file is loaded to help get the message across?

> If you consider that the overhead of OS.File is unacceptable, you should
> write the code in C++ and communicate with Dispatch.

In the ideal world, yeah. I'd like this to be an XPCOM component implemented in C++. However, Mihnea's just trying to get a test harness feature implemented, learning XPCOM/C++ is beyond the scope of that feature, and perfect is the enemy of done, so JS implementation is good for now! I was thinking that we could at least try to ensure API compatibility with the future XPCOM component, which is why I'd like to avoid OS.File, promises, and callbacks. Who knows. The consumers of this should be small, so we can always throw everything away later once something better is available.
(In reply to David Rajchenbach Teller [:Yoric] from comment #8)

> However, if it does perform main thread I/O (even something as simple as
> open/read/close), you should remember that every operation has an unbounded
> and unpredictable duration. On a busy system, this can be orders of
> magnitude worse than sending a message to a dedicated thread (seconds vs.
> milliseconds).

But isn't gps' argument (which makes sense to me) that this doesn't apply? Stuff in /proc doesn't map to anything on a physical disk (well, generally), so the usual I/O arguments don't apply. It's more akin to reading from /dev/zero or writing to /dev/null.

That said, unless there's a compelling reason not to, we should probably be in the habit of doing async file operations in every case. If for no other reason than avoiding the need to justify it, the inevitable "bugs" that will be filed about it in the future, and poor cut'n'paste habits. :)
> But isn't gps' argument (which makes sense to me) that this doesn't apply? Stuff in /proc doesn't map to anything on a physical disk (well, generally), so the usual I/O arguments don't apply. It's more akin to reading from /dev/zero or writing to /dev/null.

Well, hence the *if*. Everybody here is almost convinced that this is not the case. We just need the reassurance that we are right.
Attached patch API for reading process info (obsolete) — Splinter Review
Work in progress.
Attachment #795816 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attached patch API for reading process info (obsolete) — Splinter Review
Will leave memory for later.
Attachment #797626 - Attachment is obsolete: true
Comment on attachment 799193 [details] [diff] [review]
API for reading process info

Fire away
Attachment #799193 - Flags: feedback?(gps)
Attached patch API for reading process info (obsolete) — Splinter Review
Reindented & stuff.
Attachment #799193 - Attachment is obsolete: true
Attachment #799193 - Flags: feedback?(gps)
Attachment #799224 - Flags: feedback?(gps)
Comment on attachment 799224 [details] [diff] [review]
API for reading process info

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

Looking good!

It would be nice if the API could be more standardized between platforms. Although, there are so many subtle differences. I have a feeling you'd end with a dict with sparsely populated fields due to the different semantics.

::: toolkit/modules/ProcessInfo.jsm
@@ +32,5 @@
> +  let file = getFileFromPath(path);
> +  // open an input stream from file
> +  let istream = Cc["@mozilla.org/network/file-input-stream;1"]
> +                  .createInstance(Ci.nsIFileInputStream);
> +  istream.init(file, 0x01, 0444, 0);

You should use a constant instead of 0x01.

@@ +68,5 @@
> +                                                IO_COUNTERS.ptr); // lpIoCounters
> +
> +    let GetCurrentProcess = kernel32.declare('GetCurrentProcess',
> +                                             ctypes.winapi_abi,
> +                                             ctypes.voidptr_t); // return type

You probably want to cache some of these ctypes data structures. I'd have a setup function at module scope that returns early if things are cached.

@@ +76,5 @@
> +
> +    let data = {
> +      readBytes: parseInt(io.readBytes),
> +      writeBytes: parseInt(io.writeBytes),
> +      otherBytes: parseInt(io.otherBytes)

Always pass a second argument to parseInt.

@@ +139,5 @@
> +    let data = {
> +      user: parseInt(processedUserTime.wSecond) +
> +            parseInt(processedUserTime.wMilliseconds) / 1000,
> +      sys: parseInt(processedKernelTime.wSecond) +
> +           parseInt(processedKernelTime.wMilliseconds) / 1000

What abouts minutes, hours, days, ...?

Why can't you just do math on the FILETIME struct? It's just a big integer.

@@ +189,5 @@
> +    let sysconf = libc.declare('sysconf',
> +                               ctypes.default_abi,
> +                               ctypes.long, // return type
> +                               ctypes.int); // name
> +    let _SC_CLK_TCK = sysconf(3); // #define _SC_CLK_TCK 3

Is SC_CLK_TCK constant or can it change while the machine is running (e.g. CPU frequency is adjusted)?

@@ +207,5 @@
> +
> +let OSX = {
> +  getIOCounters: function () {
> +    // not available on OS X
> +    return {total: 0};

total: 0 implies there's a value. I'd just return an empty object or an object indicating a special state. e.g. {unavailable: true}

@@ +217,5 @@
> +    return {
> +      peak: parseInt(usage.ru_maxrss),
> +      shared: parseInt(usage.ru_ixrss),
> +      unshared_data: parseInt(usage.ru_idrss),
> +      unshared_stack: parseInt(usage.ru_isrss),

parseInt(value, base)

@@ +279,5 @@
> +  this.ProcessInfo = Linux;
> +} else if (OS.name == 'Windows_NT') {
> +  this.ProcessInfo = Windows;
> +} else {
> +  this.ProcessInfo = null;

We should probably have a dummy object that implements the API so callers don't have to check this.
Attachment #799224 - Flags: feedback?(gps) → feedback+
(In reply to Gregory Szorc [:gps] from comment #17)
> Comment on attachment 799224 [details] [diff] [review]
> API for reading process info
> 
> Review of attachment 799224 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looking good!
> 
> It would be nice if the API could be more standardized between platforms.
> Although, there are so many subtle differences. I have a feeling you'd end
> with a dict with sparsely populated fields due to the different semantics.

Well the function calls are the same, and all the returning dicts have a "total" field. Not sure we can do more than that.

> 
> ::: toolkit/modules/ProcessInfo.jsm
> @@ +32,5 @@
> > +  let file = getFileFromPath(path);
> > +  // open an input stream from file
> > +  let istream = Cc["@mozilla.org/network/file-input-stream;1"]
> > +                  .createInstance(Ci.nsIFileInputStream);
> > +  istream.init(file, 0x01, 0444, 0);
> 
> You should use a constant instead of 0x01.

Problem is, the snippet[1] uses 0x01 and the docs[2] don't list any constant with that value. Magic!

> 
> @@ +68,5 @@
> > +                                                IO_COUNTERS.ptr); // lpIoCounters
> > +
> > +    let GetCurrentProcess = kernel32.declare('GetCurrentProcess',
> > +                                             ctypes.winapi_abi,
> > +                                             ctypes.voidptr_t); // return type
> 
> You probably want to cache some of these ctypes data structures. I'd have a
> setup function at module scope that returns early if things are cached.

I don't see this as perf-sensitive code since it is enabled only to get data and that's under a mach flag, but sure, will do.


> @@ +139,5 @@
> > +    let data = {
> > +      user: parseInt(processedUserTime.wSecond) +
> > +            parseInt(processedUserTime.wMilliseconds) / 1000,
> > +      sys: parseInt(processedKernelTime.wSecond) +
> > +           parseInt(processedKernelTime.wMilliseconds) / 1000
> 
> What abouts minutes, hours, days, ...?

I could add minutes, we won't have hours days etc since we have timeouts.

> 
> Why can't you just do math on the FILETIME struct? It's just a big integer.

I was trying to stick to the API. I can do that, sure.

> 
> @@ +189,5 @@
> > +    let sysconf = libc.declare('sysconf',
> > +                               ctypes.default_abi,
> > +                               ctypes.long, // return type
> > +                               ctypes.int); // name
> > +    let _SC_CLK_TCK = sysconf(3); // #define _SC_CLK_TCK 3
> 
> Is SC_CLK_TCK constant or can it change while the machine is running (e.g.
> CPU frequency is adjusted)?

From what I read it is constant, yes.

> @@ +279,5 @@
> > +  this.ProcessInfo = Linux;
> > +} else if (OS.name == 'Windows_NT') {
> > +  this.ProcessInfo = Windows;
> > +} else {
> > +  this.ProcessInfo = null;
> 
> We should probably have a dummy object that implements the API so callers
> don't have to check this.

I'm not sure I see the problem. This is handled in the module, so a caller will only use ProcessInfo.getSomething.

[1] https://developer.mozilla.org/en-US/docs/Code_snippets/File_I_O#Line_by_line
[2] https://developer.mozilla.org/en-US/docs/XPCOM_Interface_Reference/nsIFileInputStream
BTW, I did test runs with and without resource logging on Linux (where we read on the main thread) and there was no noticeable perf difference.
Attached patch API for reading process info (obsolete) — Splinter Review
Adressed most of the comments, (together with the previous reply).

I'd rather not manually do the math on the Windows struct since
they have the API for this. I find it a bit more readable (and more..
idiomatic?).
Attachment #801955 - Flags: review?(gps)
Attachment #799224 - Attachment is obsolete: true
Comment on attachment 801955 [details] [diff] [review]
API for reading process info

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

Looking good!

Please add some basic usage docs to ProcessInfo.jsm. See Sqlite.jsm (in the same directory) for examples.

::: toolkit/modules/ProcessInfo.jsm
@@ +2,5 @@
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +this.EXPORTED_SYMBOLS = [ "ProcessInfo" ];

Nit: Cuddle braces.

@@ +40,5 @@
> +  let line = {}, lines = [], hasmore;
> +  do {
> +    hasmore = istream.readLine(line);
> +    lines.push(line.value);
> +  } while(hasmore);

This probably fails on empty lines (""), no? Do we care?

@@ +105,5 @@
> +
> +  getIOCounters: function () {
> +    this._setupCTypes();
> +    let io = new this._IO_COUNTERS();
> +    this._GetProcessIoCounters(this._GetCurrentProcess(), io.address());

Always check the return value and handle errors!

@@ +110,5 @@
> +
> +    let data = {
> +      readBytes: parseInt(io.readBytes, 10),
> +      writeBytes: parseInt(io.writeBytes, 10),
> +      otherBytes: parseInt(io.otherBytes, 10)

Might as well capture all the fields in the struct, no?

@@ +112,5 @@
> +      readBytes: parseInt(io.readBytes, 10),
> +      writeBytes: parseInt(io.writeBytes, 10),
> +      otherBytes: parseInt(io.otherBytes, 10)
> +    };
> +    data.total = data.readBytes + data.writeBytes + data.otherBytes;

data.totalBytes

@@ +127,5 @@
> +    this._GetProcessTimes(this._GetCurrentProcess(),
> +                  creationTime.address(),
> +                  exitTime.address(),
> +                  kernelTime.address(),
> +                  userTime.address());

Nit: This indenting is weird. Either align with paren or do standard 2 space indent.

@@ +142,5 @@
> +            parseInt(processedUserTime.wMilliseconds, 10) / 1000,
> +      sys: parseInt(processedKernelTime.wMinute, 10) * 60 +
> +           parseInt(processedKernelTime.wSecond, 10) +
> +           parseInt(processedKernelTime.wMilliseconds, 10) / 1000
> +    };

What about hours? I'd just perform math on the FILETIME struct. Something like:

let ticks = (kernelTime.dwHighDateTime << 32) + kernelTime.dwLowDateTime;
let milliseconds = ticks / 10000;

@@ +157,5 @@
> +
> +
> +let Linux = {
> +  _setupCTypes: function () {
> +    this._libc = ctypes.open('libc.so.6');

Can we just use libc.so? That should be symlinked to the latest libc on the system, no?

@@ +195,5 @@
> +
> +  getCPUTimes: function () {
> +    this._setupCTypes();
> +    // using ctypes to get value of _SC_CLK_TCK
> +    let _SC_CLK_TCK = this._sysconf(2); // #define _SC_CLK_TCK 2

This seems a bit fragile, but sadly it's probably the best way to do it. Can you at least throw a comment in here saying why you do it this way? Should save a bit of bugzilla foraging when someone looks at this code in the future.

@@ +202,5 @@
> +    let startPos = line.indexOf(")") + 2;
> +    let values = line.substring(startPos).split(" ");
> +    let data = {
> +      user: (values[11] | 0) / _SC_CLK_TCK,
> +      sys: (values[12] | 0) / _SC_CLK_TCK

Is there any chance the clk could be reported as 0? If so, we have a divide by 0 exception.

@@ +295,5 @@
> +  this.ProcessInfo = Linux;
> +} else if (OS.name == 'Windows_NT') {
> +  this.ProcessInfo = Windows;
> +} else {
> +  this.ProcessInfo = null;

I'd assign a dummy object that returns empty values when the public APIs are called.

::: toolkit/modules/tests/xpcshell/test_ProcessInfo.js
@@ +5,5 @@
> +
> +function run_test() {
> +  dump(JSON.stringify(ProcessInfo.getCPUTimes()) + "\n");
> +  dump(JSON.stringify(ProcessInfo.getMemoryUsage()) + "\n");
> +  dump(JSON.stringify(ProcessInfo.getIOCounters()) + "\n");

Can you add some more intelligent tests? Maybe add a sleep() or some I/O and verify counters (when present) increment.
Attachment #801955 - Flags: review?(gps) → feedback+
Attached patch API for reading process info (obsolete) — Splinter Review
* cuddled braces
* added comment for the reason we get SC_CLK_TCK
* fixed indentation in Ctypes code
Attachment #801955 - Attachment is obsolete: true
* dummy object fallback
Attachment #812612 - Attachment is obsolete: true
Sorry for the delay, been traveling lately. Comments inline.

(In reply to Gregory Szorc [:gps] from comment #21)
> Comment on attachment 801955 [details] [diff] [review]
> API for reading process info
> 
> Review of attachment 801955 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looking good!
> 
> Please add some basic usage docs to ProcessInfo.jsm. See Sqlite.jsm (in the
> same directory) for examples.

Will do.

> 
> @@ +40,5 @@
> > +  let line = {}, lines = [], hasmore;
> > +  do {
> > +    hasmore = istream.readLine(line);
> > +    lines.push(line.value);
> > +  } while(hasmore);
> 
> This probably fails on empty lines (""), no? Do we care?

We don't care. I'll try to change it to use the stream's hasAvailable (or whatever it's called) method.

> 
> @@ +105,5 @@
> > +
> > +  getIOCounters: function () {
> > +    this._setupCTypes();
> > +    let io = new this._IO_COUNTERS();
> > +    this._GetProcessIoCounters(this._GetCurrentProcess(), io.address());
> 
> Always check the return value and handle errors!

Hm, this is the same behavior as in TelemetryPing.js. I'm not sure we have to do all this for this module, but if you think we really should then I will. I'm afraid the ctype parts of the code will become horrifying.

> 
> @@ +110,5 @@
> > +
> > +    let data = {
> > +      readBytes: parseInt(io.readBytes, 10),
> > +      writeBytes: parseInt(io.writeBytes, 10),
> > +      otherBytes: parseInt(io.otherBytes, 10)
> 
> Might as well capture all the fields in the struct, no?

Fixed.

> 
> @@ +112,5 @@
> > +      readBytes: parseInt(io.readBytes, 10),
> > +      writeBytes: parseInt(io.writeBytes, 10),
> > +      otherBytes: parseInt(io.otherBytes, 10)
> > +    };
> > +    data.total = data.readBytes + data.writeBytes + data.otherBytes;
> 
> data.totalBytes

Ok, will do. The point was to have a "total" field in all the platform-specific implementation so that we could have a common way to look at the data across platforms (i.e. the web viewer). I could change the names to totalBytes and totalTime, np.

> @@ +142,5 @@
> > +            parseInt(processedUserTime.wMilliseconds, 10) / 1000,
> > +      sys: parseInt(processedKernelTime.wMinute, 10) * 60 +
> > +           parseInt(processedKernelTime.wSecond, 10) +
> > +           parseInt(processedKernelTime.wMilliseconds, 10) / 1000
> > +    };
> 
> What about hours? I'd just perform math on the FILETIME struct. Something
> like:
> 
> let ticks = (kernelTime.dwHighDateTime << 32) + kernelTime.dwLowDateTime;
> let milliseconds = ticks / 10000;

From MDN: "The operands of all bitwise operators are converted to signed 32-bit integers "
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Bitwise_Operators

I don't think we need hours.. but I can add it, it's just a two line change.

> 
> @@ +157,5 @@
> > +
> > +
> > +let Linux = {
> > +  _setupCTypes: function () {
> > +    this._libc = ctypes.open('libc.so.6');
> 
> Can we just use libc.so? That should be symlinked to the latest libc on the
> system, no?

I remember having trouble, will check again, need a linux laptop handy. Basically you get libc.so.VER_NUMBER symlinked to your lib path.
http://stackoverflow.com/questions/6495817/why-glibc-binary-is-called-libc-so-6-not-a-libc-so-1-or-libc-so-4

> @@ +202,5 @@
> > +    let startPos = line.indexOf(")") + 2;
> > +    let values = line.substring(startPos).split(" ");
> > +    let data = {
> > +      user: (values[11] | 0) / _SC_CLK_TCK,
> > +      sys: (values[12] | 0) / _SC_CLK_TCK
> 
> Is there any chance the clk could be reported as 0? If so, we have a divide
> by 0 exception.

I haven't found any docs saying that it could be zero/subunitary.

> 
> ::: toolkit/modules/tests/xpcshell/test_ProcessInfo.js
> @@ +5,5 @@
> > +
> > +function run_test() {
> > +  dump(JSON.stringify(ProcessInfo.getCPUTimes()) + "\n");
> > +  dump(JSON.stringify(ProcessInfo.getMemoryUsage()) + "\n");
> > +  dump(JSON.stringify(ProcessInfo.getIOCounters()) + "\n");
> 
> Can you add some more intelligent tests? Maybe add a sleep() or some I/O and
> verify counters (when present) increment.

Absolutely, will do.
Flags: needinfo?(gps)
Comment on attachment 812613 [details] [diff] [review]
API for reading process info

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

Bug 926990 strongly suggests that reading /proc/meminfo and should really be done on the main thread.
Consequently, I very strongly suggest an async API.
Attachment #812613 - Flags: feedback-
I see your point, however I still think that only for benchmarking/monitoring reasons it would be fine sync. I do understand that we have no control over where the code would end up being used after it lands in the tree. :(
I believe that you have two possibilities:
- either pref-off this API (and pref it back on in the test suite); or
- make this API async.

Otherwise, we are going to end up with add-ons (or worse, front-end code) that misuses it.
Or we just make it a testing-only JSM that is only distributed in test archives.
Flags: needinfo?(gps)
That works for me, too.
Whichever of the three you guys prefer.
(In reply to Gregory Szorc [:gps] from comment #28)
> Or we just make it a testing-only JSM that is only distributed in test
> archives.

How do we do achieve this?
See TESTING_JS_MODULES in services/healthreport/Makefile.in.
You need to log in before you can comment on or make changes to this bug.