Closed Bug 1333391 Opened 3 years ago Closed 3 years ago

Move GetCPUCount to threading layer

Categories

(Core :: JavaScript Engine, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: lth, Assigned: lth)

Details

Attachments

(1 file)

For background see bug 1333059.  The JS engine does not use PR_GetProcessorCount, because it does not use NSPR, instead it has its own definition (GetCPUCount) tucked away in the garbage collector.

Let's move that to js/src/threading so that when the threading layer is lifted into MFBT the GetCPUCount mechanism also appears in MFBT, to potentially benefit all who are currently using their own mechanisms.
(Try runs for this are attached to bug 1333059.)
Attachment #8829831 - Flags: review?(luke)
Comment on attachment 8829831 [details] [diff] [review]
bug1333391-getcpucount.patch

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

The actual patch seems fine, but I'd wait a bit before landing to see if anyone else involved in js/threading or mfbt wants to comment.
Attachment #8829831 - Flags: review?(luke) → review+
(In reply to Lars T Hansen [:lth] from comment #0)
> Let's move that to js/src/threading so that when the threading layer is
> lifted into MFBT the GetCPUCount mechanism also appears in MFBT, to
> potentially benefit all who are currently using their own mechanisms.

I was thinking about just moving the mutex/condvar implementations themselves; I wasn't going to attempt a full |hg mv|, as there's JS-specific stuff in the mutex/condvar bits that should remain in the JS engine itself.  I don't have any objection to landing something like GetCPUCount, but just note that it may not quickly move into MFBT/mozglue.
(In reply to Nathan Froyd [:froydnj] from comment #3)
> 
> I was thinking about just moving the mutex/condvar implementations
> themselves; I wasn't going to attempt a full |hg mv|, as there's JS-specific
> stuff in the mutex/condvar bits that should remain in the JS engine itself. 
> I don't have any objection to landing something like GetCPUCount, but just
> note that it may not quickly move into MFBT/mozglue.

Noted.  Putting it into threading/ at least makes it somewhat more visible; getting it moved to mfbt will have to remain our ambition, for now.
https://hg.mozilla.org/mozilla-central/rev/2e13f21663bb
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.