Skip to content

Commit 30778d1

Browse files
Connor ClarkDevtools-frontend LUCI CQ
authored andcommitted
[RPP] Apply optimizations to statsForTimeRange
This trace is of the performance panel: https://trace.cafe/t/zP4zA7sNLb. One thing of interest is that `statsForTimeRange` takes 3.5s, which is 33% of the `loadingComplete` callback. One thing that jumped out to me is that 33% of `statsForTimeRange` is spent inside `eventTimingsMilliSeconds`. The work this method must do is simple, but it calls another method in an effort to DRY the code. This creates an extra object allocation. Additionally, it uses the timing tag and conversion functions multiple times. Given these are called 100,000s of times during the processing of this trace, that adds up. Un-DRYing the code and should have a good effect. Additionally, `statsForTimeRange` can get away with calling `eventTimingsMicroseconds` instead of `eventTimingsMilliSeconds`, which saves 3 divisions per trace event. Bug: None Change-Id: Ie7ace912ebaac6c879233a809223710264081ed0 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6154635 Reviewed-by: Paul Irish <[email protected]> Commit-Queue: Connor Clark <[email protected]>
1 parent 2a31e37 commit 30778d1

File tree

3 files changed

+13
-31
lines changed

3 files changed

+13
-31
lines changed

front_end/models/trace/helpers/Timing.test.ts

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -64,18 +64,6 @@ describeWithEnvironment('Timing helpers', () => {
6464
});
6565
});
6666

67-
it('eventTimingsSeconds returns the right numbers', async () => {
68-
const event = {
69-
ts: 100_000, // 100k microseconds = 100ms = 0.1second
70-
dur: 50_000, // 50k microseconds = 50ms = 0.05second
71-
} as unknown as Trace.Types.Events.Event;
72-
assert.deepEqual(Trace.Helpers.Timing.eventTimingsSeconds(event), {
73-
startTime: Trace.Types.Timing.Seconds(0.1),
74-
endTime: Trace.Types.Timing.Seconds(0.15),
75-
duration: Trace.Types.Timing.Seconds(0.05),
76-
});
77-
});
78-
7967
describe('timeStampForEventAdjustedByClosestNavigation', () => {
8068
it('can use the navigation ID to adjust the time correctly', async function() {
8169
const {parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev.json.gz');

front_end/models/trace/helpers/Timing.ts

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -79,25 +79,16 @@ export interface EventTimingsData<
7979

8080
export function eventTimingsMicroSeconds(event: Types.Events.Event): EventTimingsData<Types.Timing.MicroSeconds> {
8181
return {
82-
startTime: event.ts,
83-
endTime: Types.Timing.MicroSeconds(event.ts + (event.dur ?? Types.Timing.MicroSeconds(0))),
84-
duration: Types.Timing.MicroSeconds(event.dur || 0),
82+
startTime: event.ts as Types.Timing.MicroSeconds,
83+
endTime: (event.ts + (event.dur ?? 0)) as Types.Timing.MicroSeconds,
84+
duration: (event.dur || 0) as Types.Timing.MicroSeconds,
8585
};
8686
}
8787
export function eventTimingsMilliSeconds(event: Types.Events.Event): EventTimingsData<Types.Timing.MilliSeconds> {
88-
const microTimes = eventTimingsMicroSeconds(event);
8988
return {
90-
startTime: microSecondsToMilliseconds(microTimes.startTime),
91-
endTime: microSecondsToMilliseconds(microTimes.endTime),
92-
duration: microSecondsToMilliseconds(microTimes.duration),
93-
};
94-
}
95-
export function eventTimingsSeconds(event: Types.Events.Event): EventTimingsData<Types.Timing.Seconds> {
96-
const microTimes = eventTimingsMicroSeconds(event);
97-
return {
98-
startTime: microSecondsToSeconds(microTimes.startTime),
99-
endTime: microSecondsToSeconds(microTimes.endTime),
100-
duration: microSecondsToSeconds(microTimes.duration),
89+
startTime: (event.ts / 1000) as Types.Timing.MilliSeconds,
90+
endTime: (event.ts + (event.dur ?? 0)) / 1000 as Types.Timing.MilliSeconds,
91+
duration: (event.dur || 0) / 1000 as Types.Timing.MilliSeconds,
10192
};
10293
}
10394

front_end/panels/timeline/TimelineUIUtils.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2066,9 +2066,9 @@ export class TimelineUIUtils {
20662066
},
20672067
parsedTrace: Trace.Handlers.Types.ParsedTrace, event: Trace.Types.Events.Event): boolean {
20682068
const events = parsedTrace.Renderer?.allTraceEntries || [];
2069-
const {startTime, endTime} = Trace.Helpers.Timing.eventTimingsMilliSeconds(event);
2069+
const {startTime, endTime} = Trace.Helpers.Timing.eventTimingsMicroSeconds(event);
20702070
function eventComparator(startTime: number, e: Trace.Types.Events.Event): number {
2071-
const {startTime: eventStartTime} = Trace.Helpers.Timing.eventTimingsMilliSeconds(e);
2071+
const {startTime: eventStartTime} = Trace.Helpers.Timing.eventTimingsMicroSeconds(e);
20722072
return startTime - eventStartTime;
20732073
}
20742074

@@ -2081,7 +2081,7 @@ export class TimelineUIUtils {
20812081
if (endTime) {
20822082
for (let i = index; i < events.length; i++) {
20832083
const nextEvent = events[i];
2084-
const {startTime: nextEventStartTime} = Trace.Helpers.Timing.eventTimingsMilliSeconds(nextEvent);
2084+
const {startTime: nextEventStartTime} = Trace.Helpers.Timing.eventTimingsMicroSeconds(nextEvent);
20852085
if (nextEventStartTime >= endTime) {
20862086
break;
20872087
}
@@ -2105,7 +2105,10 @@ export class TimelineUIUtils {
21052105
for (const categoryName in total) {
21062106
aggregatedTotal += total[categoryName];
21072107
}
2108-
total['idle'] = Math.max(0, endTime - startTime - aggregatedTotal);
2108+
2109+
const deltaInMillis =
2110+
Trace.Helpers.Timing.microSecondsToMilliseconds((endTime - startTime) as Trace.Types.Timing.MicroSeconds);
2111+
total['idle'] = Math.max(0, deltaInMillis - aggregatedTotal);
21092112
}
21102113
return false;
21112114
}

0 commit comments

Comments
 (0)