Bug 1726675 Comment 1 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

(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 event 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 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 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.
(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 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 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.
(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 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.
(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.

Back to Bug 1726675 Comment 1