1630872 - Replace BlocksRingBuffer with ProfileChunkedBuffer

Status: RESOLVED FIXED

(Core :: Gecko Profiler, task, P2)

Description

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

After bug 1626837, ProfileChunkedBuffer is complete and can now replace BlocksRingBuffer.

Comment 1

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1630872 - BlockRingBuffer Put* functions provide a Maybe<ProfileBufferEntryWriter> - r?canaltinova!,gregtatum

Instead of a potentially-null pointer to a ProfileBufferEntryWriter, we are
now providing a Maybe<ProfileBufferEntryWriter>, which is safer.

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1630872 - ProfileChunkedBuffer Put* functions provide a Maybe<ProfileBufferEntryWriter> - r?canaltinova!,gregtatum

Same as with BlocksRingBuffer: Instead of a potentially-null pointer to a
ProfileBufferEntryWriter, we are now providing a
Maybe<ProfileBufferEntryWriter>, which is safer.

Depends on D71286

Comment 3

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1630872 - Replace InChunkBuffer::operator bool() and operator!() with IsNull() - r?canaltinova!,gregtatum

explicit operator bool() and operator!() were cute ways to make InChunkBuffer quack like a pointer when testing if it's effectively null.
But after some experience, and since InChunkPointer will not be used in generic code where pointers would be accepted, I now think that it's better to be clearer about it and use an explicit IsNull().

Depends on D71287

Comment 4

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1630872 - Removed 'BlocksRingBuffer' from some Gecko Profiler platform variables - r?canaltinova!,gregtatum

Renamed some variables to be more generic. Their type is going to change in the next patch, and that type doesn't need to be in the names; also it will make the next patch easier to review.

Depends on D71499

Comment 5

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1630872 - Replace uses of BlocksRingBuffer with ProfileChunkedBuffer in Gecko Profiler - r?canaltinova!,gregtatum

Depends on D71500

Comment 6

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1630872 - Quick-moving InChunkPointer with `ProfileBufferBlockIndex` - r?canaltinova!,gregtatum

As opposed to ProfileBufferIndex (no "Block"), ProfileBufferBlockIndex is only supposed to point at a valid block start.
If we trust this assumption, it allows for quick access to the given block index inside the buffer, as we don't need to read blocks one by one until we reach the given position.

There are still safety checks (MOZ_ASSERTs in DEBUG builds) to verify that block indices are correctly used.

Depends on D71501

Comment 7

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1630872 - Removed 'BlocksRingBuffer' from some Base Profiler platform variables - r?canaltinova!,gregtatum

Renamed some variables to be more generic. Their type is going to change in the next patch, and that type doesn't need to be in the names; also it will make the next patch easier to review.

Depends on D71501

Comment 8

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1630872 - Replace uses of BlocksRingBuffer with ProfileChunkedBuffer in Base Profiler - r?canaltinova!,gregtatum

Depends on D71882

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/488d57eb1da7
BlockRingBuffer Put* functions provide a Maybe<ProfileBufferEntryWriter> - r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/79cc6c708645
ProfileChunkedBuffer Put* functions provide a Maybe<ProfileBufferEntryWriter> - r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/ad1644ba541d
Replace InChunkBuffer::operator bool() and operator!() with IsNull() - r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/1c27b4d14959
Removed 'BlocksRingBuffer' from some Gecko Profiler platform variables - r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/d09fce7cef49
Replace uses of BlocksRingBuffer with ProfileChunkedBuffer in Gecko Profiler - r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/7f832cccb66e
Removed 'BlocksRingBuffer' from some Base Profiler platform variables - r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/c38f6467377d
Replace uses of BlocksRingBuffer with ProfileChunkedBuffer in Base Profiler - r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/1416b0415a5e
Quick-moving InChunkPointer with `ProfileBufferBlockIndex` - r=canaltinova

Comment 10

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/488d57eb1da7
https://hg.mozilla.org/mozilla-central/rev/79cc6c708645
https://hg.mozilla.org/mozilla-central/rev/ad1644ba541d
https://hg.mozilla.org/mozilla-central/rev/1c27b4d14959
https://hg.mozilla.org/mozilla-central/rev/d09fce7cef49
https://hg.mozilla.org/mozilla-central/rev/7f832cccb66e
https://hg.mozilla.org/mozilla-central/rev/c38f6467377d
https://hg.mozilla.org/mozilla-central/rev/1416b0415a5e