For profiles with non-integer values, the values displayed in the sid…

…ebar should be properly rounded Fixes firefox-devtools#1466
julienw · Feb 25, 2019 · d7317df · d7317df
1 parent 2fb0fcb
commit d7317df
Show file tree

Hide file tree

Showing 2 changed files with 153 additions and 110 deletions.
diff --git a/src/components/sidebar/CallTreeSidebar.js b/src/components/sidebar/CallTreeSidebar.js
@@ -12,7 +12,7 @@ import {
   selectedNodeSelectors,
 } from '../../selectors/per-thread';
 import { getSelectedThreadIndex } from '../../selectors/url-state';
-import { getCategories } from '../../selectors/profile';
+import { getCategories, getProfileInterval } from '../../selectors/profile';
 import { getFunctionName } from '../../profile-logic/function-info';
 import { assertExhaustiveCheck } from '../../utils/flow';
 import CanSelectContent from './CanSelectContent';
@@ -33,6 +33,7 @@ import type {
   StackImplementation,
   TimingsForPath,
 } from '../../profile-logic/profile-data';
+import { formatMilliseconds } from '../../utils/format-numbers';
 
 type SidebarDetailProps = {|
   +label: string,
@@ -59,6 +60,7 @@ function SidebarDetail({ label, color, children }: SidebarDetailProps) {
 
 type ImplementationBreakdownProps = {|
   +breakdown: BreakdownByImplementation,
+  +isIntervalInteger: boolean,
 |};
 
 // This component is responsible for displaying the breakdown data specific to
@@ -91,7 +93,8 @@ class ImplementationBreakdown extends React.PureComponent<
   }
 
   render() {
-    const { breakdown } = this.props;
+    const { breakdown, isIntervalInteger } = this.props;
+
     const data = [];
 
     for (const implementation of this._orderedImplementations) {
@@ -106,18 +109,19 @@ class ImplementationBreakdown extends React.PureComponent<
       });
     }
 
-    return <Breakdown data={data} />;
+    return <Breakdown data={data} isIntervalInteger={isIntervalInteger} />;
   }
 }
 
 type CategoryBreakdownProps = {|
   +breakdown: BreakdownByCategory,
   +categoryList: CategoryList,
+  +isIntervalInteger: boolean,
 |};
 
 class CategoryBreakdown extends React.PureComponent<CategoryBreakdownProps> {
   render() {
-    const { breakdown, categoryList } = this.props;
+    const { breakdown, categoryList, isIntervalInteger } = this.props;
     const data = breakdown
       .map((value, categoryIndex) => {
         const category = categoryList[categoryIndex];
@@ -130,7 +134,7 @@ class CategoryBreakdown extends React.PureComponent<CategoryBreakdownProps> {
       // sort in descending order
       .sort(({ value: valueA }, { value: valueB }) => valueB - valueA);
 
-    return <Breakdown data={data} />;
+    return <Breakdown data={data} isIntervalInteger={isIntervalInteger} />;
   }
 }
 
@@ -140,19 +144,21 @@ type BreakdownProps = {|
     color?: string,
     value: Milliseconds,
   |}>,
+  +isIntervalInteger: boolean,
 |};
 
 // This stateless component is responsible for displaying the implementation
 // breakdown. It also computes the percentage from the total time.
-function Breakdown({ data }: BreakdownProps) {
+function Breakdown({ data, isIntervalInteger }: BreakdownProps) {
   const totalTime = data.reduce((result, item) => result + item.value, 0);
 
   return data.filter(({ value }) => value).map(({ group, color, value }) => {
     const percentage = Math.round(value / totalTime * 100);
+    const maxFractionalDigits = isIntervalInteger ? 0 : 1;
 
     return (
       <SidebarDetail label={group} color={color} key={group}>
-        {value}ms ({percentage}%)
+        {formatMilliseconds(value, 3, maxFractionalDigits)} ({percentage}%)
       </SidebarDetail>
     );
   });
@@ -166,13 +172,21 @@ type StateProps = {|
   +lib: string,
   +timings: TimingsForPath,
   +categoryList: CategoryList,
+  +isIntervalInteger: boolean,
 |};
 
 type Props = ConnectedProps<{||}, StateProps, {||}>;
 
 class CallTreeSidebar extends React.PureComponent<Props> {
   render() {
-    const { selectedNodeIndex, name, lib, timings, categoryList } = this.props;
+    const {
+      selectedNodeIndex,
+      name,
+      lib,
+      timings,
+      categoryList,
+      isIntervalInteger,
+    } = this.props;
     const {
       forPath: { selfTime, totalTime },
       forFunc: { selfTime: selfTimeForFunc, totalTime: totalTimeForFunc },
@@ -196,6 +210,8 @@ class CallTreeSidebar extends React.PureComponent<Props> {
       selfTimeForFunc.value / rootTime * 100
     );
 
+    const maxFractionalDigits = isIntervalInteger ? 0 : 1;
+
     return (
       <aside className="sidebar sidebar-calltree">
         <div className="sidebar-contents-wrapper">
@@ -215,17 +231,26 @@ class CallTreeSidebar extends React.PureComponent<Props> {
           </header>
           <h3 className="sidebar-title2">This selected call node</h3>
           <SidebarDetail label="Running Time">
-            {totalTime.value}ms ({totalTimePercent}%)
+            {formatMilliseconds(totalTime.value, 3, maxFractionalDigits)} ({
+              totalTimePercent
+            }%)
           </SidebarDetail>
           <SidebarDetail label="Self Time">
-            {selfTime.value ? `${selfTime.value}ms (${selfTimePercent}%)` : '—'}
+            {selfTime.value
+              ? `${formatMilliseconds(
+                  selfTime.value,
+                  3,
+                  maxFractionalDigits
+                )} (${selfTimePercent}%)`
+              : '—'}
           </SidebarDetail>
           {totalTime.breakdownByCategory ? (
             <>
               <h4 className="sidebar-title3">Categories</h4>
               <CategoryBreakdown
                 breakdown={totalTime.breakdownByCategory}
                 categoryList={categoryList}
+                isIntervalInteger={isIntervalInteger}
               />
             </>
           ) : null}
@@ -234,6 +259,7 @@ class CallTreeSidebar extends React.PureComponent<Props> {
               <h4 className="sidebar-title3">Implementation – running time</h4>
               <ImplementationBreakdown
                 breakdown={totalTime.breakdownByImplementation}
+                isIntervalInteger={isIntervalInteger}
               />
             </React.Fragment>
           ) : null}
@@ -242,25 +268,32 @@ class CallTreeSidebar extends React.PureComponent<Props> {
               <h4 className="sidebar-title3">Implementation – self time</h4>
               <ImplementationBreakdown
                 breakdown={selfTime.breakdownByImplementation}
+                isIntervalInteger={isIntervalInteger}
               />
             </React.Fragment>
           ) : null}
           <h3 className="sidebar-title2">
             This function across the entire tree
           </h3>
           <SidebarDetail label="Running Time">
-            {totalTimeForFunc.value}ms ({totalTimeForFuncPercent}%)
+            {formatMilliseconds(totalTimeForFunc.value, 3, maxFractionalDigits)}{' '}
+            ({totalTimeForFuncPercent}%)
           </SidebarDetail>
           <SidebarDetail label="Self Time">
             {selfTimeForFunc.value
-              ? `${selfTimeForFunc.value}ms (${selfTimeForFuncPercent}%)`
+              ? `${formatMilliseconds(
+                  selfTimeForFunc.value,
+                  3,
+                  maxFractionalDigits
+                )} (${selfTimeForFuncPercent}%)`
               : '—'}
           </SidebarDetail>
           {totalTimeForFunc.breakdownByImplementation ? (
             <React.Fragment>
               <h4 className="sidebar-title3">Implementation – running time</h4>
               <ImplementationBreakdown
                 breakdown={totalTimeForFunc.breakdownByImplementation}
+                isIntervalInteger={isIntervalInteger}
               />
             </React.Fragment>
           ) : null}
@@ -269,6 +302,7 @@ class CallTreeSidebar extends React.PureComponent<Props> {
               <h4 className="sidebar-title3">Implementation – self time</h4>
               <ImplementationBreakdown
                 breakdown={selfTimeForFunc.breakdownByImplementation}
+                isIntervalInteger={isIntervalInteger}
               />
             </React.Fragment>
           ) : null}
@@ -287,6 +321,7 @@ const options: ExplicitConnectOptions<{||}, StateProps, {||}> = {
     lib: selectedNodeSelectors.getLib(state),
     timings: selectedNodeSelectors.getTimingsForSidebar(state),
     categoryList: getCategories(state),
+    isIntervalInteger: Number.isInteger(getProfileInterval(state)),
   }),
   component: CallTreeSidebar,
 };