Skip to content

Commit

Permalink
RPP: remove SyntheticInvalidation
Browse files Browse the repository at this point in the history 
I was investigating this bug in LH
[GoogleChrome/lighthouse#16111] where the
trace engine OOMs, and the reporter notes that the InvalidationsHandler
map is 328MB in size.

During my initial investigation I realised we were creating synthetic
invalidations for no real purpose; all the information exists in the
events as is, and instead we can just create a type to group them and
everything gets much more simple.

I don't expect this to resolve the problem in that bug report, but it is
firstly cleaner to not create synthetic events if we don't need to, and
it does also mean we are now not going to allocate a new object per
every invalidation we want to store, so I expect this to be a positive
change.

Bug: 352680159
Change-Id: I4817774ce692c0dd73eb2156389148d7b756208c
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5702081
Auto-Submit: Jack Franklin <[email protected]>
Commit-Queue: Andres Olivares <[email protected]>
Reviewed-by: Andres Olivares <[email protected]>
  • Loading branch information
@jackfranklin
jackfranklin authored and Devtools-frontend LUCI CQ committed Jul 12, 2024
1 parent 2e2368a commit 80d1944
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 82 deletions.
10 changes: 5 additions & 5 deletions front_end/models/trace/handlers/InvalidationsHandler.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@ import type * as Protocol from '../../../generated/protocol.js';
import {TraceLoader} from '../../../testing/TraceLoader.js';
import * as TraceEngine from '../trace.js';

function invalidationDataForTestAssertion(invalidation: TraceEngine.Types.TraceEvents.SyntheticInvalidation): {
function invalidationDataForTestAssertion(invalidation: TraceEngine.Types.TraceEvents.InvalidationTrackingEvent): {
nodeId: Protocol.DOM.BackendNodeId,
nodeName?: string,
reason?: string,
stackTrace?: TraceEngine.Types.TraceEvents.TraceEventCallFrame[],
} {
return {
nodeId: invalidation.nodeId,
nodeName: invalidation.nodeName,
reason: invalidation.reason,
stackTrace: invalidation.stackTrace,
nodeId: invalidation.args.data.nodeId,
nodeName: invalidation.args.data.nodeName,
reason: invalidation.args.data.reason,
stackTrace: invalidation.args.data.stackTrace,
};
}

Expand Down
43 changes: 7 additions & 36 deletions front_end/models/trace/handlers/InvalidationsHandler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,15 @@ import {HandlerState} from './types.js';

let handlerState = HandlerState.UNINITIALIZED;

const invalidationsForEvent = new Map<Types.TraceEvents.TraceEventData, Types.TraceEvents.SyntheticInvalidation[]>();
const invalidationsForEvent =
new Map<Types.TraceEvents.TraceEventData, Types.TraceEvents.InvalidationTrackingEvent[]>();

let lastRecalcStyleEvent: Types.TraceEvents.TraceEventUpdateLayoutTree|null = null;

// Used to track paints so we track invalidations correctly per paint.
let hasPainted = false;

const allInvalidationTrackingEvents:
Array<Types.TraceEvents.TraceEventScheduleStyleInvalidationTracking|
Types.TraceEvents.TraceEventStyleRecalcInvalidationTracking|Types.TraceEvents
.TraceEventStyleInvalidatorInvalidationTracking|Types.TraceEvents.TraceEventLayoutInvalidationTracking> =
[];
const allInvalidationTrackingEvents: Array<Types.TraceEvents.InvalidationTrackingEvent> = [];

export function reset(): void {
handlerState = HandlerState.UNINITIALIZED;
Expand All @@ -38,32 +35,9 @@ export function initialize(): void {
}

function addInvalidationToEvent(
event: Types.TraceEvents.TraceEventData,
invalidation: Types.TraceEvents.TraceEventScheduleStyleInvalidationTracking|
Types.TraceEvents.TraceEventStyleRecalcInvalidationTracking|
Types.TraceEvents.TraceEventStyleInvalidatorInvalidationTracking|
Types.TraceEvents.TraceEventLayoutInvalidationTracking): void {
event: Types.TraceEvents.TraceEventData, invalidation: Types.TraceEvents.InvalidationTrackingEvent): void {
const existingInvalidations = invalidationsForEvent.get(event) || [];

const syntheticInvalidation: Types.TraceEvents.SyntheticInvalidation = {
...invalidation,
name: 'SyntheticInvalidation',
frame: invalidation.args.data.frame,
nodeId: invalidation.args.data.nodeId,
rawEvent: invalidation,
};

if (invalidation.args.data.nodeName) {
syntheticInvalidation.nodeName = invalidation.args.data.nodeName;
}
if (invalidation.args.data.reason) {
syntheticInvalidation.reason = invalidation.args.data.reason;
}
if (invalidation.args.data.stackTrace) {
syntheticInvalidation.stackTrace = invalidation.args.data.stackTrace;
}

existingInvalidations.push(syntheticInvalidation);
existingInvalidations.push(invalidation);
invalidationsForEvent.set(event, existingInvalidations);
}

Expand All @@ -88,10 +62,7 @@ export function handleEvent(event: Types.TraceEvents.TraceEventData): void {
return;
}

if (Types.TraceEvents.isTraceEventScheduleStyleInvalidationTracking(event) ||
Types.TraceEvents.isTraceEventStyleRecalcInvalidationTracking(event) ||
Types.TraceEvents.isTraceEventStyleInvalidatorInvalidationTracking(event) ||
Types.TraceEvents.isTraceEventLayoutInvalidationTracking(event)) {
if (Types.TraceEvents.isTraceEventInvalidationTracking(event)) {
if (hasPainted) {
// If we have painted, then we can clear out the list of all existing
// invalidations, as we cannot associate them across frames.
Expand Down Expand Up @@ -150,7 +121,7 @@ export async function finalize(): Promise<void> {
}

interface InvalidationsData {
invalidationsForEvent: Map<Types.TraceEvents.TraceEventData, Types.TraceEvents.SyntheticInvalidation[]>;
invalidationsForEvent: Map<Types.TraceEvents.TraceEventData, Types.TraceEvents.InvalidationTrackingEvent[]>;
}

export function data(): InvalidationsData {
Expand Down
9 changes: 6 additions & 3 deletions front_end/models/trace/helpers/Trace.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import * as Common from '../../../core/common/common.js';
import * as Platform from '../../../core/platform/platform.js';
import type * as Protocol from '../../../generated/protocol.js';
import type * as CPUProfile from '../../cpu_profile/cpu_profile.js';
import * as Types from '../types/types.js';

Expand All @@ -25,9 +26,6 @@ type MatchingPairableAsyncEvents = {
* indiscriminately.
*/
function stackTraceForEvent(event: Types.TraceEvents.TraceEventData): Types.TraceEvents.TraceEventCallFrame[]|null {
if (Types.TraceEvents.isSyntheticInvalidation(event)) {
return event.stackTrace || null;
}
if (event.args?.data?.stackTrace) {
return event.args.data.stackTrace;
}
Expand Down Expand Up @@ -622,3 +620,8 @@ export function eventHasCategory(event: Types.TraceEvents.TraceEventData, catego
}
return parsedCategoriesForEvent.has(category);
}

export function nodeIdForInvalidationEvent(event: Types.TraceEvents.InvalidationTrackingEvent):
Protocol.DOM.BackendNodeId|null {
return event.args.data.nodeId ?? null;
}
18 changes: 6 additions & 12 deletions front_end/models/trace/types/TraceEvents.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1660,19 +1660,13 @@ export function isTraceEventActivateLayerTree(event: TraceEventData): event is T
return event.name === KnownEventName.ActivateLayerTree;
}

export interface SyntheticInvalidation extends TraceEventInstant {
name: 'SyntheticInvalidation';
nodeName?: string;
rawEvent: TraceEventScheduleStyleInvalidationTracking|TraceEventStyleRecalcInvalidationTracking|
TraceEventStyleInvalidatorInvalidationTracking|TraceEventLayoutInvalidationTracking;
nodeId: Protocol.DOM.BackendNodeId;
frame: string;
reason?: string;
stackTrace?: TraceEventCallFrame[];
}
export type InvalidationTrackingEvent =
TraceEventScheduleStyleInvalidationTracking|TraceEventStyleRecalcInvalidationTracking|
TraceEventStyleInvalidatorInvalidationTracking|TraceEventLayoutInvalidationTracking;

export function isSyntheticInvalidation(event: TraceEventData): event is SyntheticInvalidation {
return event.name === 'SyntheticInvalidation';
export function isTraceEventInvalidationTracking(event: TraceEventData): event is InvalidationTrackingEvent {
return isTraceEventScheduleStyleInvalidationTracking(event) || isTraceEventStyleRecalcInvalidationTracking(event) ||
isTraceEventStyleInvalidatorInvalidationTracking(event) || isTraceEventLayoutInvalidationTracking(event);
}

export interface TraceEventDrawLazyPixelRef extends TraceEventInstant {
Expand Down
14 changes: 8 additions & 6 deletions front_end/panels/timeline/TimelineUIUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1840,7 +1840,7 @@ export class TimelineUIUtils {
}

private static async generateInvalidationsList(
invalidations: TraceEngine.Types.TraceEvents.SyntheticInvalidation[],
invalidations: TraceEngine.Types.TraceEvents.InvalidationTrackingEvent[],
contentHelper: TimelineDetailsContentHelper): Promise<void> {
const {groupedByReason, backendNodeIds} = TimelineComponents.DetailsView.generateInvalidationsList(invalidations);

Expand All @@ -1857,19 +1857,21 @@ export class TimelineUIUtils {
}

private static generateInvalidationsForReason(
reason: string, invalidations: TraceEngine.Types.TraceEvents.SyntheticInvalidation[],
reason: string, invalidations: TraceEngine.Types.TraceEvents.InvalidationTrackingEvent[],
relatedNodesMap: Map<number, SDK.DOMModel.DOMNode|null>|null, contentHelper: TimelineDetailsContentHelper): void {
function createLinkForInvalidationNode(invalidation: TraceEngine.Types.TraceEvents.SyntheticInvalidation):
function createLinkForInvalidationNode(invalidation: TraceEngine.Types.TraceEvents.InvalidationTrackingEvent):
HTMLSpanElement {
const node = (invalidation.nodeId && relatedNodesMap) ? relatedNodesMap.get(invalidation.nodeId) : null;
const node = (invalidation.args.data.nodeId && relatedNodesMap) ?
relatedNodesMap.get(invalidation.args.data.nodeId) :
null;
if (node) {
const nodeSpan = document.createElement('span');
void Common.Linkifier.Linkifier.linkify(node).then(link => nodeSpan.appendChild(link));
return nodeSpan;
}
if (invalidation.nodeName) {
if (invalidation.args.data.nodeName) {
const nodeSpan = document.createElement('span');
nodeSpan.textContent = invalidation.nodeName;
nodeSpan.textContent = invalidation.args.data.nodeName;
return nodeSpan;
}
const nodeSpan = document.createElement('span');
Expand Down
40 changes: 20 additions & 20 deletions front_end/panels/timeline/components/DetailsView.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,59 +169,59 @@ export function buildRowsForWebSocketEvent(
* It is exported only for testing purposes.
**/
export function generateInvalidationsList(
invalidations: TraceEngine.Types.TraceEvents.SyntheticInvalidation[],
invalidations: TraceEngine.Types.TraceEvents.InvalidationTrackingEvent[],
): {
groupedByReason: Record<string, TraceEngine.Types.TraceEvents.SyntheticInvalidation[]>,
groupedByReason: Record<string, TraceEngine.Types.TraceEvents.InvalidationTrackingEvent[]>,
backendNodeIds: Set<Protocol.DOM.BackendNodeId>,
} {
const groupedByReason: Record<string, TraceEngine.Types.TraceEvents.SyntheticInvalidation[]> = {};
const groupedByReason: Record<string, TraceEngine.Types.TraceEvents.InvalidationTrackingEvent[]> = {};

const backendNodeIds: Set<Protocol.DOM.BackendNodeId> = new Set();
for (const invalidation of invalidations) {
backendNodeIds.add(invalidation.nodeId);
backendNodeIds.add(invalidation.args.data.nodeId);

let reason = invalidation.reason || 'unknown';
let reason = invalidation.args.data.reason || 'unknown';

// ScheduleStyle events do not always have a reason, but if they tell us
// via their data what changed, we can update the reason that we show to
// the user.
if (reason === 'unknown' &&
TraceEngine.Types.TraceEvents.isTraceEventScheduleStyleInvalidationTracking(invalidation.rawEvent) &&
invalidation.rawEvent.args.data.invalidatedSelectorId) {
switch (invalidation.rawEvent.args.data.invalidatedSelectorId) {
TraceEngine.Types.TraceEvents.isTraceEventScheduleStyleInvalidationTracking(invalidation) &&
invalidation.args.data.invalidatedSelectorId) {
switch (invalidation.args.data.invalidatedSelectorId) {
case 'attribute':
reason = 'Attribute';
if (invalidation.rawEvent.args.data.changedAttribute) {
reason += ` (${invalidation.rawEvent.args.data.changedAttribute})`;
if (invalidation.args.data.changedAttribute) {
reason += ` (${invalidation.args.data.changedAttribute})`;
}
break;
case 'class':
reason = 'Class';
if (invalidation.rawEvent.args.data.changedClass) {
reason += ` (${invalidation.rawEvent.args.data.changedClass})`;
if (invalidation.args.data.changedClass) {
reason += ` (${invalidation.args.data.changedClass})`;
}
break;
case 'id':
reason = 'Id';
if (invalidation.rawEvent.args.data.changedId) {
reason += ` (${invalidation.rawEvent.args.data.changedId})`;
if (invalidation.args.data.changedId) {
reason += ` (${invalidation.args.data.changedId})`;
}
break;
}
}

if (reason === 'PseudoClass' &&
TraceEngine.Types.TraceEvents.isTraceEventStyleRecalcInvalidationTracking(invalidation.rawEvent) &&
invalidation.rawEvent.args.data.extraData) {
TraceEngine.Types.TraceEvents.isTraceEventStyleRecalcInvalidationTracking(invalidation) &&
invalidation.args.data.extraData) {
// This will append the `:focus` onto the reason.
reason += invalidation.rawEvent.args.data.extraData;
reason += invalidation.args.data.extraData;
}

if (reason === 'Attribute' &&
TraceEngine.Types.TraceEvents.isTraceEventStyleRecalcInvalidationTracking(invalidation.rawEvent) &&
invalidation.rawEvent.args.data.extraData) {
TraceEngine.Types.TraceEvents.isTraceEventStyleRecalcInvalidationTracking(invalidation) &&
invalidation.args.data.extraData) {
// Append the attribute that changed.
reason += ` (${invalidation.rawEvent.args.data.extraData})`;
reason += ` (${invalidation.args.data.extraData})`;
}

if (reason === 'StyleInvalidator') {
Expand Down

0 comments on commit 80d1944

Please sign in to comment.