Closed Bug 1721939 Opened 3 years ago Closed 3 years ago

Implement new thread registration classes

Categories

(Core :: Gecko Profiler, task, P2)

task

Tracking

()

RESOLVED FIXED
93 Branch
Tracking Status
firefox93 --- fixed

People

(Reporter: mozbugz, Assigned: mozbugz)

References

Details

Attachments

(6 files)

To allow on-thread stack walking without locking (as much), we need access to some RegisteredThread data without locks, e.g., the JSContext.

This bug will introduce new classes that contain the same data, but will stronger type-checked access to that data.
For example, the JSContext pointer is only modified on the thread, so it should be possible to access it on that thread without any lock. But we also want to force other thread (like the Profiler's sampler thread) to use a proper lock before accessing it.

Note that these will not be used just yet, a follow-up bug will replace RegisteredThread with the new classes, and another bug will use their new abilities and remove unnecessary locking during on-thread sampling.

ProfilerThreadRegistrationInfo will replace ThreadInfo, and contains thread-specific information that may be kept after that thread has ended, to identify recorded profiling data about that thread.
It is public and not ref-counted because it will be included as value into the thread registration data (to avoid a separate allocation).

This class will contain platform-specific data about threads.
It needs to be public because it will be included as value into the thread registration data (to avoid a separate allocation).

Depends on D120813

This class contains the same data as was in RacyRegisteredThread and RegisteredThread, but this data is kept protected, accessors will be added through sub-classes in the next patch, and tests in a later patch once publicly accessible.

Note that ThreadRegistrationInfo, ProfilingStack, and PlatformData are now directly included as values.

Depends on D120814

Non-virtual sub-classes of ProfilerThreadRegistrationData provide layers of public accessors to subsets of the data.
Each level builds on the previous one and adds further access to more data, but always with the appropriate guards where necessary.
These classes have protected constructors, so only some trusted classes (in later patches) will be able to construct them, and then give limited access depending on who asks (the owning thread or another one), and how much data they actually need.

The hierarchy is, from base to most derived:

  • ThreadRegistrationData (previous patch)
  • ThreadRegistrationUnlockedConstReader
  • ThreadRegistrationUnlockedConstReaderAndAtomicRW
  • ThreadRegistrationUnlockedRWForLockedProfiler
  • ThreadRegistrationUnlockedReaderAndAtomicRWOnThread
  • ThreadRegistrationLockedRWFromAnyThread
  • ThreadRegistrationLockedRWOnThread
  • ThreadRegistration::EmbeddedData (next patch, as data member of ThreadRegistration)

Tech detail: These classes need to be a single hierarchy so that the upcoming ThreadRegistration class can contain the most-derived class, and from there can publish references to base classes without relying on Undefined Behavior. (It's not allowed to have some object and give a reference to a sub-class, unless that object was really constructed as that sub-class at least, even if that sub-class only adds member functions!)
And where appropriate, these references will come along with the required lock.

For example:
JSContext can only be written on the owning thread, through ThreadRegistrationLockedRWOnThread (which will be accessed through TLS = Thread Local Storage), which requires a per-thread lock.
Thanks to that, JSContext can be read using ThreadRegistrationUnlockedReaderAndAtomicRWOnThread, without lock because we're on the owning thread, so there cannot be any write at the same time.
Other threads will be able to read it as well, but they will only see it through ThreadRegistrationLockedRWFromAnyThread, which will require the per-thread lock, thereby preventing simultaneous writes.

Depends on D120815

This class is how a thread will register itself, and contains the thread's relevant data.
A registration-accessor object can be accessed on the thread with GetOnThreadPtr() or WithOnThreadRef{,Or}(), and from there some of the data accessor may be obtained, with per-thread lock where necessary.

(The next patch will introduce ThreadRegistry to access registrations from other threads.)

Depends on D120816

mozilla::profiler::ThreadRegistry keeps a list of all ThreadRegistrations, and makes it safely accessible from any thread.

While a thread is accessing the list, that list cannot be changed (a thread (un)registering itself needs to acquire the associated mutex). In particular, a thread will not disappear during that time, so it's safe to access the ThreadRegistration data that is owned by these threads.

The subset of ThreadRegistrationData accessor sub-classes available through ThreadRegistry is different from direct on-thread access, to guarantee safe access in different circumstances.
For example, JSContext may be read through a LockedRWFromAnyThread, which requires the per-thread lock, thereby preventing any simultaneous JSContext change from the owning thread. And because the LockedRWOnThread accessor is not reachable through the registry, it's not allowed, or even possible, to change JSContext from other threads.

Depends on D120817

Attachment #9233046 - Attachment description: Bug 1721939 - ProfilerThreadRegistrationData.cpp and accessor sub-classes - r?canaltinova → Bug 1721939 - ProfilerThreadRegistrationData accessor sub-classes - r?canaltinova
Pushed by gsquelart@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/42c847ad2d5d ProfilerThreadRegistrationInfo - r=canaltinova https://hg.mozilla.org/integration/autoland/rev/12bb9027578e ProfilerThreadPlatformData - r=canaltinova https://hg.mozilla.org/integration/autoland/rev/13410ce7b2a1 ProfilerThreadRegistrationData - r=canaltinova https://hg.mozilla.org/integration/autoland/rev/7d79b208426a ProfilerThreadRegistrationData accessor sub-classes - r=canaltinova https://hg.mozilla.org/integration/autoland/rev/794001b9d204 ProfilerThreadRegistration - r=canaltinova https://hg.mozilla.org/integration/autoland/rev/f60fb636e214 ProfilerThreadRegistry - r=canaltinova
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: