1333655 - Improve the profiler's Thread class

Status: RESOLVED FIXED

(Core :: Gecko Profiler, defect)

Description

[Nicholas Nethercote [:njn] (On Long Service Leave)]

It's a clumsy cross-platform abstraction. GetCurrentId() and tid_t are the only
genuinely cross-platform things in it, and the patch keeps those in place.

The remaining methods are not implemented on all platforms (none are
implemented on Linux) and the fields are all used on either Mac or Windows, but
not both. So the patch introduces MacOSThread and Win32Thread classes and moves
the relevant Thread method implementations and fields into them. There is
sufficiently little overlap between the two new classes that there is no point
sharing a base class between them.

The patch also changes some of the existing code to use Gecko style, e.g.
|mFoo| instead of |foo_| for class fields.

Comment 1

[Nicholas Nethercote [:njn] (On Long Service Leave)]

Attachment: Improve the profiler's Thread class

Comment 2

[Markus Stange [:mstange]]

Comment on attachment 8830125 [details] [diff] [review]
Improve the profiler's Thread class

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

Ok. Without the standalone build requirement, there's not much reason not to use PR_CreateThread and friends, is there?

Comment 3

[Nicholas Nethercote [:njn] (On Long Service Leave)]

Attachment: Improve the profiler's Thread class

Sorry that I'm asking for re-review, but I realized that Win32Thread and
MacOSThread both had a single sub-class (two different versions of
SamplerThread) and that this sub-classing is unnecessary. Removing it gets rid
of some virtual methods and simplifies thread naming on Mac.

Comment 4

[Nicholas Nethercote [:njn] (On Long Service Leave)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/31af743f4a0273ad198f50fe8d20dd6978027979
Bug 1333655 - Improve the profiler's Thread class. r=mstange.

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/31af743f4a02