1204554 - Add environment variable to spew LCOV info into a file.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicolas B. Pierron [:nbp | PTO until 11/11]]

Current LCOV output is only available in the JS Shell, through a testing function.  We should make this output available for compartments within Firefox.

The simplest approach is to use an environment variable which is used when the compartments are finalized, and use it to append the LCOV output to the output file.

Comment 2

[Nicolas B. Pierron [:nbp | PTO until 11/11]]

Hum … this is probably not as easy as I thought in the first place.

I was thinking of using the destructor of the JSCompartment, but unfortunately this destructor is called by the sweeping phase of the GC, which implies that we have no JSContext available, and that all the scripts are probably garbage collected too.

I think we could manage to make a version of the code coverage which does not rely on the JSContext, but then we would have to fake the function body of LazyScripts.

The alternative I am thinking of, is to use the enterCompartmentDepth flag, and an additional flag which records the fact that we have new executed code since the we entered the compartment, and then collect the code coverage each time we leave the top-level entry of each compartment.  Then when the compartment is destroyed, we spew everything into a file.

This solution should work, but it will probably add a lot of overhead on the execution of callbacks, as we would execute a few functions, and collect the coverage of all scripts.

Terrence, do you know if there is another way to be able to access a compartment for spilling the code coverage of all its JSScript at the end of its live-time, and in which condition I can safely delazify LazyScripts (have a JSContext) in order to have precise code locations and branches?

Comment 3

[Nicolas B. Pierron [:nbp | PTO until 11/11]]

Ok, after discussing with Terrence, we may have an option to avoid generating the Lcov output all the time, which is a good news.

The idea would be to use LcovWriteScript[1] in the finalizer of the JSScript (the JSContext argument is not used) for each JSScript and to store these LSprinter strings on the compartment.

Then when the compartment is finalized, we will aggregate all information including the the name[2] of the compartment and the content produced by the script finalizer for each source file[3].  This content would then be written into a file which should be hold by the JSRuntime, such that other compartments from the same runtime can reuse the same file.

In order to avoid locking around the file, when we have multiple runtimes (each with its own thread) and multiples processes.  We should use the date, process-id and the thread-id to name the generated lcov files.

[1] https://dxr.mozilla.org/mozilla-central/source/js/src/jsopcode.cpp#1990
[2] https://dxr.mozilla.org/mozilla-central/source/js/src/jsopcode.cpp#2142-2159
[3] https://dxr.mozilla.org/mozilla-central/source/js/src/jsopcode.cpp#2207-2231

Comment 4

[Nicolas B. Pierron [:nbp | PTO until 11/11]]

Attachment: part 1 - Remove unused JSContext from LcovWriteScript.

Comment 5

[Nicolas B. Pierron [:nbp | PTO until 11/11]]

Attachment: part 2 - Split LCov functions to make the aggregation of results incremental.

This patch move LCOV outpuyt generation in its own file such that we can use
the structure to aggregate results in the JSCompartment, when JSScript are
finalized, and also to aggregate results with the JS API function.

As the finalizer do not have any JSContext*, we have to keep the traversal
in GenerateLcovInfo to call getOrCreateScript before serializing the
bytecode of all JSScripts.

Comment 6

[Nicolas B. Pierron [:nbp | PTO until 11/11]]

Attachment: part 3 - WIP: Collect lcov output on the JSCompartment, and on the JSRuntime.

This patch adds an LcovRuntime class which is used to open the output file
if needed.

TODO:
 - Disable LazyScripts when global LCOV is enabled.

 - The ScriptSourceObject seems to be moved to the tenured space
 (0x4b4b4b4b) when we are trying to read it from the JSScript::finalize
 function. I will look if it is possible to register the source/script
 presence when they are created as part of the current compartment, and then
 fill the gap when they are finalized.

Comment 7

[Terrence Cole [:terrence]]

Comment on attachment 8663681 [details] [diff] [review]
part 2 - Split LCov functions to make the aggregation of results incremental.

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

Looks fine to me.

::: js/src/vm/CodeCoverage.cpp
@@ +73,5 @@
> +
> +    outBRDA_.exportInto(out);
> +    out.printf("BRF:%d\n", numBranchesFound_);
> +    out.printf("BRH:%d\n", numBranchesHit_);
> +    

Extra whitespace.

Comment 8

[Brian Hackett [Laid off!]]

Comment on attachment 8663681 [details] [diff] [review]
part 2 - Split LCov functions to make the aggregation of results incremental.

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

::: js/src/vm/CodeCoverage.cpp
@@ +277,5 @@
> +    // Skip any operation if we already some out-of memory issues.
> +    if (outTN_.hadOutOfMemory())
> +        return;
> +
> +    // On the first call, write the compartment name, and allocate 

more extra whitespace

@@ +330,5 @@
> +    outTN_.put("TN:");
> +    if (rt->compartmentNameCallback) {
> +        char name[1024];
> +        {
> +            // Hazard analysis cannot tell that the callback do not GC.

does not

Comment 9

[Nicolas B. Pierron [:nbp | PTO until 11/11]]

As I am trying to move the collection of the code coverage to the GC, I faced the PCCount JSFriendAPI.  This API is basically holding everything alive until it is queried.

The ScriptCountsMap which is on the JSCompartment has a strong references on the JSScript and on the ScriptCount.  Making this a weak reference will imply that the PCCount API will no longer be able to report information about GCed scripts from one compartment.  This implies that any profiled-script / compartment is held alive until the end of the JSRuntime.  This does not sounds practical if we intend to collect code coverage on the test suite of Firefox.

As they hold a strong reference, we have to remove them before the beginning of the collection started by the JSRuntime destructor.  Unfortunately, removing them implies that we no longer have any code coverage information when JSScript::finalize is called, which is unfortunate, as we are trying to collect it.

The idea would be to change the ScriptCountsMap and to replace it by a WeakMap.  (1) Doing so implies that the PCCount API will not work on dead script / compartment any more.  (2) Otherwise, we would have to convert the PCCount API to collect information in the same way as we do for LCov, by aggregating serialized results on the JSRuntime.  Thus the runtime will hold strings instead of the JSScript and the ScriptCounts.  (3) A Third solution, would be to implement Bug 1190792, and remove the PCCount JSFriendAPI completely.

Brian, what do you think?
I would highly prefer if we can go with the first solution, as this would reduce the amount of work blocking this issue.

Comment 10

[Brian Hackett [Laid off!]]

(In reply to Nicolas B. Pierron [:nbp] from comment #9)
> As I am trying to move the collection of the code coverage to the GC, I
> faced the PCCount JSFriendAPI.  This API is basically holding everything
> alive until it is queried.
> 
> The ScriptCountsMap which is on the JSCompartment has a strong references on
> the JSScript and on the ScriptCount.  Making this a weak reference will
> imply that the PCCount API will no longer be able to report information
> about GCed scripts from one compartment.  This implies that any
> profiled-script / compartment is held alive until the end of the JSRuntime. 
> This does not sounds practical if we intend to collect code coverage on the
> test suite of Firefox.
> 
> As they hold a strong reference, we have to remove them before the beginning
> of the collection started by the JSRuntime destructor.  Unfortunately,
> removing them implies that we no longer have any code coverage information
> when JSScript::finalize is called, which is unfortunate, as we are trying to
> collect it.
> 
> The idea would be to change the ScriptCountsMap and to replace it by a
> WeakMap.  (1) Doing so implies that the PCCount API will not work on dead
> script / compartment any more.  (2) Otherwise, we would have to convert the
> PCCount API to collect information in the same way as we do for LCov, by
> aggregating serialized results on the JSRuntime.  Thus the runtime will hold
> strings instead of the JSScript and the ScriptCounts.  (3) A Third solution,
> would be to implement Bug 1190792, and remove the PCCount JSFriendAPI
> completely.
> 
> Brian, what do you think?
> I would highly prefer if we can go with the first solution, as this would
> reduce the amount of work blocking this issue.

(1) is fine but it would be nice if scripts with PC counts were kept alive if StartPCCountProfiling has been called (I'm assuming you won't be using this API for code coverage), to preserve the API's behavior.  This could be done by having an additional vector of scripts on the runtime which hold strong references during GC, is filled in when making script counts and StartPCCountProfiling has been called, and is cleared when StopPCCountProfiling is called.

Comment 11

[Nicolas B. Pierron [:nbp | PTO until 11/11]]

Attachment: part 3 - Collect lcov output on the JSCompartment, and on the JSRuntime.

Add code in JSScript::finalize to aggregate the content of the code coverage
on the compartment first, and then write it into a file which is kept open
by the JSRuntime.

Comment 12

[Nicolas B. Pierron [:nbp | PTO until 11/11]]

Attachment: part 3.1 - Prevent lazy parsing if we have to spew lcov result.

As we cannot parse JSScript while we collect the code coverage under
JSScript::finalize, we prevent the generation of any LazyScript.

This way the assertion under fun.nonLazyScript() should not trigger.

Comment 13

[Nicolas B. Pierron [:nbp | PTO until 11/11]]

Attachment: part 3.2 - Collect the source files before any script, as they are swept first.

ScriptSource are finalized before the JSScript which are using them.  This
patch change the way we aggregate sources to ensure that we extract the
filename only when the source is available.

Then top-level JSScript is associated with its corresponding ScriptSource
object by looking at the raw pointer behind the sourceObject_ field of the
JSScript.  We have to use the raw pointer, as we can no longer unwrap it
any-more, since it got garbage collected.

Comment 14

[Nicolas B. Pierron [:nbp | PTO until 11/11]]

Attachment: part 3.3 - Only collect inner JSScript if they have the same source.

When an eval is used in the body of a function, the script of the eval seems
to have a reference to the parent script.  The previous iteration scheme
caused JSScript which already got finalized to be processed a second time
with the writeScript method.

The writeScript method is using the compartment pointer of the JSScript to
locate the code coverage information.  This access failed before the pointer
was poisoned.

This patch prevent the parent script to be visited a second time, by
checking the sourceObject_ field.  If the sourceObject_ field is identical,
then we can queue the function, otherwise it is likely already finalized and
it will not match the raw pointer saved in the LCovSource structure.

Comment 15

[Nicolas B. Pierron [:nbp | PTO until 11/11]]

Attachment: part 3.4 - Ensure that scriptCountsMaps data are still alive until the destruction of compartments.

This patch converts the ScriptCountsMap into a weak-map, such that the
enabling global code coverage does not hold all JSScript & JSCompartment
until the end of the JSRuntime.

As it does not longer add a strong reference, we no longer have the
issue relate to the destruction of the JSRuntime which clean the
scriptCountsMap instances before the finalize function calls.

The problem might still exists if we ever want to enable the --dump-bytecode
output at the same time as the global code coverage, but this should not be
a big problem.

Comment 16

[Terrence Cole [:terrence]]

Comment on attachment 8666834 [details] [diff] [review]
part 3 - Collect lcov output on the JSCompartment, and on the JSRuntime.

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

::: js/src/jsscript.cpp
@@ +2936,5 @@
>      // may have created it and partially initialized it with
>      // JSScript::Create(), but not yet finished initializing it with
>      // fullyInitFromEmitter() or fullyInitTrivial().
>  
> +    // Collect code coverage information of this script and all its inner

s/of/for/

The phrasing "of this script" implies that the coverage is a property of the script itself, whereas "for the script" indicates than it is something that happens to or around the script on its behalf.

::: js/src/vm/CodeCoverage.cpp
@@ +6,5 @@
>  
>  #include "vm/CodeCoverage.h"
>  
> +#include <stdio.h>
> +#include <unistd.h>

I'm pretty sure including "unistd" is not going to work on windows. This should probably make use of either PR_GetCurrentThread or PRProcessAttr or somesuch. Or else move this file to a different part of moz.build, depending on how we plan to use the functionality.

@@ +360,5 @@
>  }
>  
> +LCovRuntime::LCovRuntime()
> +  : out_(),
> +    pid_(getpid())

As above, I don't think getpid is available in windows unless you are running under a special unix mode.

@@ +388,5 @@
> +        fprintf(stderr, "Warning: LCovRuntime::init: Cannot serialize file name.");
> +        return;
> +    }
> +
> +    // If we cannot open the file, then report a warning.

Drop either the comma or "then".

::: js/src/vm/CodeCoverage.h
@@ +111,5 @@
> +
> +    // If the environment variable JS_CODE_COVERAGE_OUTPUT_DIR is set to a
> +    // directory, create a file inside this directory which uses the process
> +    // ID, the thread ID and a timestamp to ensure the uniqueness of the
> +    // file.

This is true, but focuses on the "how" to the complete exclusion of "what". It creates a unique file. Is there anything in that file that I might care about?

@@ +122,5 @@
> +    // into a file.
> +    void writeLCovResult(const JSRuntime* runtime, LCovCompartment& comp);
> +
> +  private:
> +    // When a process fork, the file will remain open, but 2 processes will

Disturbingly backward as English grammer is, this should be: "When a process fork*s*".

@@ +123,5 @@
> +    void writeLCovResult(const JSRuntime* runtime, LCovCompartment& comp);
> +
> +  private:
> +    // When a process fork, the file will remain open, but 2 processes will
> +    // have the same file. To avoid having conflicting writes, we re-open a

Drop "having"; it's unneeded.

s/re-open/open/ The file is hopefully not open yet or it's still going to have conflicts.

@@ +128,5 @@
> +    // new file for the child process.
> +    void maybeReopenAfterFork();
> +
> +  private:
> +    // Output file which is created if the code coverage is enabled.

s/the code coverage/code coverage/

@@ +131,5 @@
> +  private:
> +    // Output file which is created if the code coverage is enabled.
> +    Fprinter out_;
> +
> +    // Used to watch for fork of the current process. When the process fork,

Disturbingly backward as English grammer is, this should be: "When the process fork*s*".

@@ +132,5 @@
> +    // Output file which is created if the code coverage is enabled.
> +    Fprinter out_;
> +
> +    // Used to watch for fork of the current process. When the process fork,
> +    // we want to close the current file and open a new open.

"open a new open".

"Used to watch for..." is missing a subject. How about: The process' PID is used to watch for...".

::: js/src/vm/Runtime.h
@@ +1055,5 @@
>  
>      /* Strong references on scripts held for PCCount profiling API. */
>      JS::PersistentRooted<js::ScriptAndCountsVector>* scriptAndCountsVector;
>  
> +    /* Code coverage output */

Add a period at the end.

Comment 17

[Terrence Cole [:terrence]]

Comment on attachment 8666841 [details] [diff] [review]
part 3.1 - Prevent lazy parsing if we have to spew lcov result.

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

I assume you meant to flag Brian for this review.

Comment 18

[Terrence Cole [:terrence]]

Comment on attachment 8666843 [details] [diff] [review]
part 3.2 - Collect the source files before any script, as they are swept first.

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

This turned out to be surprisingly clean!

::: js/src/jsscript.cpp
@@ +1508,5 @@
>  {
>      ScriptSourceObject* sso = &obj->as<ScriptSourceObject>();
> +
> +    // If code coverage is enabled, then record the filename associated with
> +    // this source object.

s/then//

::: js/src/vm/CodeCoverage.cpp
@@ +339,5 @@
> +{
> +    if (!sources_)
> +        return nullptr;
> +
> +    // Lookup the first matching source.

Perhaps s/Lookup/Find/.

@@ +412,5 @@
>  void
>  LCovRuntime::init(const JSRuntime* runtime)
>  {
>      const char* outDir = getenv("JS_CODE_COVERAGE_OUTPUT_DIR");
> +    if (!outDir || *outDir == 0)

Oh, good find! Please migrate this hunk up to the LCovRuntime patch unless you plan to fold all of this before landing.

::: js/src/vm/CodeCoverage.h
@@ +31,5 @@
>  {
>    public:
> +    explicit LCovSource(LifoAlloc* alloc, JSObject* sso);
> +
> +    // Wether the given script source object match this LCovSource.

s/match/matches/

lol English.

@@ +103,5 @@
>    private:
>      // Write the script name in out.
>      bool writeCompartmentName(JSCompartment* comp);
>  
> +    // Return the LCovSource entry which match the given ScriptSourceObject.

s/match/matches/

Comment 19

[Terrence Cole [:terrence]]

Comment on attachment 8666846 [details] [diff] [review]
part 3.3 - Only collect inner JSScript if they have the same source.

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

::: js/src/vm/CodeCoverage.cpp
@@ +126,5 @@
>              if (!fun.isInterpreted())
>                  continue;
>              MOZ_ASSERT(!fun.isInterpretedLazy());
>  
> +            // Eval script can refer to their parent script, in order to extend

s/script/scripts/
s/,//

@@ +127,5 @@
>                  continue;
>              MOZ_ASSERT(!fun.isInterpretedLazy());
>  
> +            // Eval script can refer to their parent script, in order to extend
> +            // their scope.  We only care about the inner function which are in

s/function which/functions, which/

@@ +132,5 @@
> +            // the same source, and which are assumed to be visited in the same
> +            // order as the source content.
> +            //
> +            // Note: It is possible that the JSScript visited here has already
> +            // been finalized, in which case the sourceObject() would be a

s/would be/will be/

@@ +133,5 @@
> +            // order as the source content.
> +            //
> +            // Note: It is possible that the JSScript visited here has already
> +            // been finalized, in which case the sourceObject() would be a
> +            // poisoned pointer.

Please add something like: "This is safe because all scripts are currently finalized in the foreground."

Comment 20

[Terrence Cole [:terrence]]

Comment on attachment 8666857 [details] [diff] [review]
part 3.4 - Ensure that scriptCountsMaps data are still alive until the destruction of compartments.

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

::: js/src/jscompartment.cpp
@@ +574,5 @@
>      if (objectMetadataTable)
>          objectMetadataTable->trace(trc);
>  
> +    // If code coverage is only enabled with the Debugger or the LCovOutput,
> +    // then the following comment holds.

s/with the/by the/

@@ +576,5 @@
>  
> +    // If code coverage is only enabled with the Debugger or the LCovOutput,
> +    // then the following comment holds.
> +    //
> +    // The scriptCountsMap maps weak-reference of JSScript pointers to

s/reference of/references to/

There are multiple references in the table, so use the plural form. The correct proposition is "to", since the references target the JSScript, whereas "of" is the other way around.

@@ +577,5 @@
> +    // If code coverage is only enabled with the Debugger or the LCovOutput,
> +    // then the following comment holds.
> +    //
> +    // The scriptCountsMap maps weak-reference of JSScript pointers to
> +    // ScriptCounts structure. It uses a HashMap instead of a WeakMap, such that

s/structure/structures/
More than one ScriptCounts can be held, so use the plural form.

s/such that/so that/
The "such" proposition generally precedes a verb + noun phrase with a new target. We can use "so" instead to take the target noun in context (the subject in this case), as you've done here.

@@ +578,5 @@
> +    // then the following comment holds.
> +    //
> +    // The scriptCountsMap maps weak-reference of JSScript pointers to
> +    // ScriptCounts structure. It uses a HashMap instead of a WeakMap, such that
> +    // we can keep the data alive for the JSScript::finalize call. Thus we do

s/Thus/Thus,/
The introductory phrase, in this case just a proposition, requires an trailing comma.

@@ +579,5 @@
> +    //
> +    // The scriptCountsMap maps weak-reference of JSScript pointers to
> +    // ScriptCounts structure. It uses a HashMap instead of a WeakMap, such that
> +    // we can keep the data alive for the JSScript::finalize call. Thus we do
> +    // not trace the keys of the HashMap to avoid adding a strong reference on

s/on/to/
The strong reference is owned by the HashMap, not the JSScript, so we need the invert the direction of the proposition.

@@ +581,5 @@
> +    // ScriptCounts structure. It uses a HashMap instead of a WeakMap, such that
> +    // we can keep the data alive for the JSScript::finalize call. Thus we do
> +    // not trace the keys of the HashMap to avoid adding a strong reference on
> +    // the JSScript pointers, but on the other hand we ensure that they are not
> +    // moved in JSCompartment::fixupAfterMovingGC.

Both the proposition "but" and the connective phrase "on the other hand" are typically used to indicate a reversal, whereas fixupAfterMovingGC is more of a supplement to the functionality above rather than a contradiction. I would make this a separate sentence and use a different connective:

"... the JSScript pointers. Additionally, we assert that the JSScripts have not been moved in JSCompartment::fixupAfterMovingGC."

@@ +585,5 @@
> +    // moved in JSCompartment::fixupAfterMovingGC.
> +    //
> +    // If the code coverage is either enabled with the --dump-bytecode command
> +    // line option, or with the PCCount JSFriend API functions, then we mark the
> +    // keys of the map, to hold the JSScript alive.

s/,//
"to hold the JSScript alive" is a propositional phrase (note the beginning proposition) and not a "normal" subject+verb phrase. Propositional phrases do not need to be joined with a comma.

@@ +771,5 @@
>      fixupInitialShapeTable();
>      objectGroups.fixupTablesAfterMovingGC();
> +
> +#ifdef DEBUG
> +    // Assert that none of the JSScript pointers, which are used as key of the

s/key/keys/

The "are used as keys of the scriptCountsMap HashMap" sub-clause breaks the flow of the sentence and makes it hard to remember that the target of "moved" is the JSScript pointers. Let's set this clause out with em-dash instead of a proposition: "Assert that none of the JSScript pointers -- keys of the scriptCountsMap HashMap -- are moved."

@@ +774,5 @@
> +#ifdef DEBUG
> +    // Assert that none of the JSScript pointers, which are used as key of the
> +    // scriptCountsMap HashMap are moved.  We do not mark these keys because we
> +    // need weak references.  We do not use a WeakMap because these entries
> +    // would be collected before the JSScript::finalize calls which is used to summarized the content of the code coverage.

This line looks to be longer than 80 chars.

Also, please use 1 space between sentences throughout.

@@ +1004,5 @@
>          }
>          return;
>      }
>  
> +    // If any other mean to enable code coverage is still enabled, keep it.

How about: "If code coverage is enabled by any other means, keep it."

::: js/src/jsscript.h
@@ +491,5 @@
>  
> +// Note: The key of this hash map is a weak reference to a JSScript.  We do not
> +// use the WeakMap implementation provided in jsweakmap.h because it would be
> +// collected at the beginning of the sweeping of the compartment, thus before
> +// the call to the JSScript::finalize functions which is used to aggregate code

s/is/are/

Comment 21

[Brian Hackett [Laid off!]]

Comment on attachment 8666834 [details] [diff] [review]
part 3 - Collect lcov output on the JSCompartment, and on the JSRuntime.

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

::: js/src/vm/CodeCoverage.cpp
@@ +382,5 @@
> +    size_t tid = runtime->ownerThreadNative();
> +
> +    char name[1024];
> +    size_t len = JS_snprintf(name, sizeof(name), "%s/%" PRId64 "-%d-%d.info",
> +                             outDir, timestamp, size_t(pid_), tid);

I don't know how the DOM threads implementation works atm, but if there is a thread pool rather than a one-thread-per-DOM-worker ironclad guarantee then these files can end up aliasing for different runtimes.  It might be better to just have a global Atomic<size_t> or something which can be increment-and-fetch-new-value'ed when you need a process-wide unique value.

Comment 22

[Jeff Walden [:Waldo]]

(In reply to Brian Hackett (:bhackett) from comment #21)
> I don't know how the DOM threads implementation works atm, but if there is a
> thread pool rather than a one-thread-per-DOM-worker ironclad guarantee then
> these files can end up aliasing for different runtimes.  It might be better
> to just have a global Atomic<size_t> or something which can be
> increment-and-fetch-new-value'ed when you need a process-wide unique value.

Bug 1128091 had an implementation of approximately this.  It hadn't landed, tho, because on 32-bit systems it's conceivable, if unlikely, that we could wrap around.  Maybe it's okay to just crash in that case.

Comment 23

[Nicolas B. Pierron [:nbp | PTO until 11/11]]

(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #22)
> (In reply to Brian Hackett (:bhackett) from comment #21)
> > […]  It might be better
> > to just have a global Atomic<size_t> or something which can be
> > increment-and-fetch-new-value'ed when you need a process-wide unique value.
> 
> Bug 1128091 had an implementation of approximately this.  It hadn't landed,
> tho, because on 32-bit systems it's conceivable, if unlikely, that we could
> wrap around.  Maybe it's okay to just crash in that case.

I will go for the atomic counter, but as I included a timestamp which is renewed each time we call the init function, I do not think that we have to worry about overflow.

I think it is unlikely that we can create 2^32 JSRuntime in less than 1 second.

Comment 24

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/aa410adc30c0
https://hg.mozilla.org/integration/mozilla-inbound/rev/6d0e0a5cfb4b
https://hg.mozilla.org/integration/mozilla-inbound/rev/673f622280ed
https://hg.mozilla.org/integration/mozilla-inbound/rev/35247eec9d61
https://hg.mozilla.org/integration/mozilla-inbound/rev/c403924d9a60
https://hg.mozilla.org/integration/mozilla-inbound/rev/c9575abbf46e
https://hg.mozilla.org/integration/mozilla-inbound/rev/e5e97faa6d1d

Comment 25

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/aa410adc30c0
https://hg.mozilla.org/mozilla-central/rev/6d0e0a5cfb4b
https://hg.mozilla.org/mozilla-central/rev/673f622280ed
https://hg.mozilla.org/mozilla-central/rev/35247eec9d61
https://hg.mozilla.org/mozilla-central/rev/c403924d9a60
https://hg.mozilla.org/mozilla-central/rev/c9575abbf46e
https://hg.mozilla.org/mozilla-central/rev/e5e97faa6d1d

Comment 26

[Sylvestre Ledru [:Sylvestre]]

I wonder if we should not document that somewhere.

Comment 27

[Nicolas B. Pierron [:nbp | PTO until 11/11]]

(In reply to Sylvestre Ledru [:sylvestre] from comment #26)
> I wonder if we should not document that somewhere.

Where would you expect it to be documented?

Comment 28

[Florian Scholz (Open Web Docs)]

(In reply to Nicolas B. Pierron [:nbp] from comment #27)
> (In reply to Sylvestre Ledru [:sylvestre] from comment #26)
> > I wonder if we should not document that somewhere.
> 
> Where would you expect it to be documented?

A new page about code coverage could be added to https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey

Also, it looks like the documentation written in bug 1176880 (about Debugger.Script.getOffsetsCoverage) has not yet been published to https://developer.mozilla.org/en-US/docs/Tools/Debugger-API/Debugger.Script