Closed Bug 1096666 Opened 7 years ago Closed 7 years ago

measure time spent in each compartment

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla37

People

(Reporter: blassey, Assigned: blassey)

References

Details

Attachments

(1 file)

No description provided.
Attachment #8520287 - Flags: review?(wmccloskey)
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.
Assignee: nobody → blassey.bugs
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.
Attachment #8520287 - Flags: review?(wmccloskey) → review+
https://hg.mozilla.org/mozilla-central/rev/8a22f1bacf72
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
You need to log in before you can comment on or make changes to this bug.