Closed Bug 1552063 Opened 6 months ago Closed 5 months ago

Use power-of-2 capacity in GeckoProfiler.h and internally

Categories

(Core :: Gecko Profiler, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox68 --- wontfix
firefox69 --- fixed

People

(Reporter: gerald, Assigned: gerald)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

The buffer capacity is used as given (from MOZ_PROFILER_STARTUP_ENTRIES or the add-on) almost everywhere, until it is finally rounded up to a power of two when actually allocating the buffer.

This can be a bit confusing (see bug 1543407) so I would suggest we use powers of 2 everywhere possible, from the profiler API down to all internal storages.

A simple PowerOf2 class should do the trick, that can be set with an arbitrary value but then will always report a power of two. We can then replace all instances of capacity with it.

Separately, bug 1543407 can be kept to coordinate the UI work (either making it snap to powers of 2 by itself, or communicating between UI and profiler core).

PowerOfTwo stores a power of 2 value, i.e., 2^N.
PowerOfTwoMask stores a mask corresponding to a power of 2, i.e., 2^N-1.

These should be used in places where a power of 2 (or its mask) is stored or
expected.
% PowerOfTwo{,Mask} and & PowerOfTwoMask operations are optimal.

MakePowerOfTwo{,Mask}<T, Value>() may be used to create statically-checked
constants.

{,Make}PowerOfTwo{,Mask}{32,64} shortcuts for common 32- and 64-bit types.

PowerOfTwo makes for a cleaner and more expressive interface, showing that the
profiler will use a power-of-2 storage size.

Using PowerOfTwoMask in ProfilerBuffer also makes it more obvious that we want
cheap modulo operations.
And we don't need to keep the original capacity, as it's only used once and can
easily be recomputed from the mask.

Depends on D36026

Pushed by gsquelart@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6284bcd7304e
PowerOfTwo, PowerOfTwoMask - r=gregtatum
https://hg.mozilla.org/integration/autoland/rev/2975f3f76576
Use PowerOfTwo and PowerOfTwoMask in profilers - r=gregtatum

Thanks Andreea.

The failure in devtools/server/tests/browser/browser_perf-04.j is also caused by this bug:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=253822432&repo=autoland&lineNumber=19640

Flags: needinfo?(gsquelart)
Pushed by gsquelart@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/03db1aa367a2
PowerOfTwo, PowerOfTwoMask - r=gregtatum
https://hg.mozilla.org/integration/autoland/rev/6f075fcdd821
Use PowerOfTwo and PowerOfTwoMask in profilers - r=gregtatum
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
You need to log in before you can comment on or make changes to this bug.