Open Bug 1726675 Opened 3 years ago Updated 2 years ago

Try to reduce the space taken by decimal numbers in JSON profile

Categories

(Core :: Gecko Profiler, task, P3)

task

Tracking

()

People

(Reporter: mozbugz, Unassigned)

References

(Blocks 1 open bug)

Details

Looking at JSON profiles, there are a lot of decimal values (mostly timestamps or durations) with many digits that just take precious space for little benefit, e.g.:
361.65560000000005
363.03499999999997
0.16666666666666666

There are a range of issues, and different ways to solve them:

  • Unneeded precision. These numbers being relative times (in milliseconds), a precision better than nanoseconds (fast processors may fit 3-4 cycles per nanosecond) is probably wasted space. So we could already round the above numbers to 361.655600, 363.035000, 0.166667.
  • Non-rounding: Sometimes we get these almost rounded values like 0.999999, would it be acceptable and possible to really round them?
  • Binary (source data) -> decimal (JSON) -> binary (back to JS) -> decimal (display) conversion issues. This is a hard problem (I think), because fixing one step may be undone at the next step! Ideally we'd want to keep the source data as-is all the way through, and only convert it and round it for friendly display. But we're probably stuck with JSON numbers in profiles, so I'm not sure how much can be done and if it's worth the effort? One possible solution would be to use appropriate units so that we only deal with lossless integer numbers (e.g., 361655600 ns), but then we'd lose the ability to save space by removing trailing decimal zeroes.

One thing I was thinking of doing, is to write which units are used, like I've started with profile.meta.sampleUnits.time="ms". And if the front-end could process and handle these given units, this could provide a migration path to later switch to e.g. "ns", which could help solve some of the above issues.

(In reply to Gerald Squelart [:gerald] (he/him) from comment #0)

One possible solution would be to use appropriate units so that we only deal with lossless integer numbers (e.g., 361655600 ns),

I think this would be a good solution.

but then we'd lose the ability to save space by removing trailing decimal zeroes.

That's true, but we rarely have integer microseconds or even integer milliseconds in the profile values. I'd say a lot less than 1% of the float millisecond values in typical profiles have fewer than 6 digits after the decimal point. And the vast majority currently have more than 10 digits after the decimal point.

One thing I was thinking of doing, is to write which units are used, like I've started with profile.meta.sampleUnits.time="ms". And if the front-end could process and handle these given units, this could provide a migration path to later switch to e.g. "ns", which could help solve some of the above issues.

I am against a generic approach. We already have a migration path: profile versioning and upgraders. I think the profile format should determine the units, and any necessary conversion should be handled by anything that outputs the profile format. This has the following benefits:

  • The units can be documented right on the type declaration.
  • All timestamp variables in the Firefox profiler code will always have a known unit; the unit can even be part of the variable name.
  • Supporting only one unit minimizes complexity in the Firefox profiler code. No dynamic conversions are needed, e.g. when comparing to other time values or when displaying values to the user.

+1 for using ns everywhere without mentioning in the meta ;-)

Suggested by Florian at Toronto 2022 work week, I think it fits in this bug:
Reduce the number of digits for eventdelay entries https://searchfox.org/mozilla-central/rev/e567185fa464270f94430e7cf62d134f4df9a69f/tools/profiler/core/ProfileBufferEntry.cpp#581,584

(Though we may completely abandon these eventdelays aka responsiveness soon-ish. 😄)

You need to log in before you can comment on or make changes to this bug.