Closed Bug 1574143 Opened Last month Closed Last month

BlocksRingBuffer: Make Reader and EntryWriter RAII, remove EntryReserver

Categories

(Core :: Gecko Profiler, task, P2)

task

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox70 --- fixed

People

(Reporter: gerald, Assigned: gerald)

References

(Blocks 1 open bug)

Details

Attachments

(3 files, 1 obsolete file)

Based on further experience with BlocksRingBuffer, I think we should:

  • Make Reader and EntryWriter RAII, to avoid potential issues with moving/copying/reassigning; also it's easier to reason about its lifetime relative to mutexes.
  • Remove EntryReserver, as it's just a layer that's not needed in the end (At the beginning I had some use, in particular to group writes together, but that need has evaporated).

The main goal of these bugs is to move markers to a new storage, so I'm adding
lots of markers to TestBaseProfiler.
Also adding labels, easier to read unsymbolicated profiles, and gives a bit more
coverage too.

EntryWriter doesn't even need to be moveable, as BlocksRingBuffer can just
create one on the stack, and pass it by reference to callbacks.
This removes risks, and potential data copies.

Depends on D42114

The point of the EntryReserver was mainly to have an object that represented a
writing lock on BlocksRingBuffer, so potentially perform multiple consecutive
writes.
After some experience implementing bug 1562604, there's actually no need for it.

Depends on D42115

In practice the Reader doesn't need to be copied/moved/reassign.
BlocksRingBuffer::Read() can just instantiate one on the stack, and pass it by
reference to callbacks.

Depends on D42116

Attachment #9085733 - Attachment is obsolete: true
Pushed by gsquelart@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1050dca058c3
Make BlocksRingBuffer::EntryWriter RAII - r=gregtatum
https://hg.mozilla.org/integration/autoland/rev/2fd28586dfbe
Remove BlocksRingBuffer::EntryReserver - r=gregtatum
https://hg.mozilla.org/integration/autoland/rev/e54b58ba5f3a
Make BlocksRingBuffer::Reader RAII - r=gregtatum
Blocks: 1574821
You need to log in before you can comment on or make changes to this bug.