Skip to content

Commit 8e11a7d

Browse files
Connor ClarkDevtools-frontend LUCI CQ
authored andcommitted
[RPP] Minor performance fix for entity map update function
This function is at the top of the hot path for loading a trace. The real time being spent is within third-party-web's getDomainFromOriginOrURL. This CL is a very minor improvement, to reduce repeated map lookups (.has followed by .get). Bug: 40278532 Change-Id: I6025590061f2327f29296efd56e9de4b8c24c98e Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6219689 Commit-Queue: Connor Clark <[email protected]> Reviewed-by: Paul Irish <[email protected]>
1 parent 93edd20 commit 8e11a7d

File tree

3 files changed

+19
-20
lines changed

3 files changed

+19
-20
lines changed

front_end/models/trace/handlers/NetworkRequestsHandler.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -486,7 +486,7 @@ export async function finalize(): Promise<void> {
486486
requestsByTime.push(networkEvent);
487487
requestsById.set(networkEvent.args.data.requestId, networkEvent);
488488
// Update entity relationships for network events.
489-
HandlerHelpers.updateEventForEntities(networkEvent, entityMappings);
489+
HandlerHelpers.addEventToEntityMapping(networkEvent, entityMappings);
490490
const initiatorUrl = networkEvent.args.data.initiator?.url ||
491491
Helpers.Trace.getZeroIndexedStackTraceForEvent(networkEvent)?.at(0)?.url;
492492
if (initiatorUrl) {

front_end/models/trace/handlers/RendererHandler.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ export function buildHierarchy(
356356
// Update the entryToNode map with the entries from this thread
357357
for (const [entry, node] of treeData.entryToNode) {
358358
entryToNode.set(entry, node);
359-
HandlerHelpers.updateEventForEntities(entry, entityMappings);
359+
HandlerHelpers.addEventToEntityMapping(entry, entityMappings);
360360
}
361361
}
362362
}

front_end/models/trace/handlers/helpers.ts

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -20,20 +20,16 @@ export interface EntityMappings {
2020
eventsByEntity: Map<Entity, Types.Events.Event[]>;
2121
}
2222

23-
export function getEntityForEvent(event: Types.Events.Event, entityByUrlCache: Map<string, Entity>): Entity|undefined {
23+
export function getEntityForEvent(event: Types.Events.Event, entityCache: Map<string, Entity>): Entity|undefined {
2424
const url = getNonResolvedURL(event);
2525
if (!url) {
2626
return;
2727
}
28-
return getEntityForUrl(url, entityByUrlCache);
28+
return getEntityForUrl(url, entityCache);
2929
}
3030

31-
export function getEntityForUrl(url: string, entityByUrlCache: Map<string, Entity>): Entity|undefined {
32-
if (entityByUrlCache.has(url)) {
33-
return entityByUrlCache.get(url);
34-
}
35-
const entity = ThirdPartyWeb.ThirdPartyWeb.getEntity(url) ?? makeUpEntity(entityByUrlCache, url);
36-
return entity;
31+
export function getEntityForUrl(url: string, entityCache: Map<string, Entity>): Entity|undefined {
32+
return ThirdPartyWeb.ThirdPartyWeb.getEntity(url) ?? makeUpEntity(entityCache, url);
3733
}
3834

3935
export function getNonResolvedURL(
@@ -145,15 +141,18 @@ function makeUpChromeExtensionEntity(entityCache: Map<string, Entity>, url: stri
145141
return chromeExtensionEntity;
146142
}
147143

148-
export function updateEventForEntities(entry: Types.Events.Event, entityMappings: EntityMappings): void {
149-
const entity = getEntityForEvent(entry, entityMappings.createdEntityCache);
150-
if (entity) {
151-
if (entityMappings.eventsByEntity.has(entity)) {
152-
const events = entityMappings.eventsByEntity.get(entity) ?? [];
153-
events?.push(entry);
154-
} else {
155-
entityMappings.eventsByEntity.set(entity, [entry]);
156-
}
157-
entityMappings.entityByEvent.set(entry, entity);
144+
export function addEventToEntityMapping(event: Types.Events.Event, entityMappings: EntityMappings): void {
145+
const entity = getEntityForEvent(event, entityMappings.createdEntityCache);
146+
if (!entity) {
147+
return;
158148
}
149+
150+
const events = entityMappings.eventsByEntity.get(entity);
151+
if (events) {
152+
events.push(event);
153+
} else {
154+
entityMappings.eventsByEntity.set(entity, [event]);
155+
}
156+
157+
entityMappings.entityByEvent.set(event, entity);
159158
}

0 commit comments

Comments
 (0)