Move PCCounts state from ZoneGroup back to the runtime

RESOLVED FIXED in Firefox 54

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: bhackett, Assigned: bhackett)

Tracking

Trunk
mozilla54
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox54 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
Created attachment 8833513 [details] [diff] [review]
patch

Bug 1325050 moved the PCCounts state (|profilingScripts|, |scriptAndCountsVector|) from JSRuntime to ZoneGroup.  While this is nice for guaranteeing a preemptively scheduled thread exclusive access to this state, it isn't really compatible with the PCCounts API, which expects exclusive access to the entire runtime and wants counts in the form of a single runtime wide vector.  Rather than reworking this API, it seems simpler to just go back to having the state on the runtime for now, since there isn't a problem with synchronization here in a cooperatively scheduled runtime.

Most of this patch is backing out pieces of bug 1325050.  Sorry about the churn.
Attachment #8833513 - Flags: review?(jdemooij)
Comment on attachment 8833513 [details] [diff] [review]
patch

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

When we move to running JS concurrently, will the ProtectedData asserts catch problems here?
Attachment #8833513 - Flags: review?(jdemooij) → review+
(Assignee)

Comment 2

2 years ago
(In reply to Jan de Mooij [:jandem] from comment #1)
> Comment on attachment 8833513 [details] [diff] [review]
> patch
> 
> Review of attachment 8833513 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> When we move to running JS concurrently, will the ProtectedData asserts
> catch problems here?

Yes, before we can have preemptive multithreading we'll need to remove ActiveThreadData and all other stuff that depends on having an active thread on the runtime, replacing the synchronization methods on these fields with something that behaves correctly with concurrently running JS.

Comment 4

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/392e110a9769
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox54: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.