1096666 - measure time spent in each compartment

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: measure_compartments.patch

Comment 1

[Nicholas Nethercote [inactive]]

Comment on attachment 8520287 [details] [diff] [review]
measure_compartments.patch

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

Is it possible to re-enter an already-entered compartment? If so, this will break.

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8520287 [details] [diff] [review]
measure_compartments.patch

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

r+ if you do this only for add-ons.

::: js/src/jscompartment.h
@@ +166,4 @@
>      js::ReadBarrieredGlobalObject global_;
>  
>      unsigned                     enterCompartmentDepth;
> +    PRIntervalTime               startInterval;

Please change to int64_t.

@@ +170,3 @@
>  
>    public:
> +    PRIntervalTime               totalTime;

Same here.

@@ +170,5 @@
>  
>    public:
> +    PRIntervalTime               totalTime;
> +    void enter() {
> +        if (!enterCompartmentDepth) {

Thinking about this more, I don't think this is really the right approach for generic profiling of JS running time. Entering a compartment is a really hot operation, and measuring the time here is expensive. I think a sampling-based profiler would be a much better way to do it.

That said, it's probably okay to do this for addons. If we make this branch |if (addonId && !enterCompartmentDepth)|, then it wouldn't cost us much.

@@ +171,5 @@
>    public:
> +    PRIntervalTime               totalTime;
> +    void enter() {
> +        if (!enterCompartmentDepth) {
> +            startInterval = PR_IntervalNow();

We want to be able to compile the JS engine without NSPR. Please use PRMJ_Now() instead, which is available everywhere.

@@ +178,5 @@
> +    }
> +    void leave() {
> +        enterCompartmentDepth--;
> +        if (!enterCompartmentDepth) {
> +            totalTime += (PR_IntervalNow() - startInterval);

Same here about PRMJ_Now() and checking addonId.

Comment 3

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/fx-team/rev/219d81afdd50 for the check_spidermonkey_style.py failures in https://treeherder.mozilla.org/logviewer.html#?job_id=1394787&repo=fx-team

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8a22f1bacf72