Fastesterer timeline#3576
Open
GiantRobots wants to merge 32 commits into
Open
Conversation
…it for the middle of the timeline
Each EventGroup previously held two redundant structures: an events: Map<string, WorkflowEvent> (primary storage) and an eventIds: Set<string> (for O(1) has() checks). For a 40k-event workflow this meant allocating ~20k Map objects (~700 bytes each) and ~20k Set objects (~300 bytes each) — around 20MB of overhead on top of the actual event data. Replace both with a single eventList: WorkflowEvent[] as the primary storage. Groups have 1–5 events in practice so linear search (find/some) is O(1), and array push is cheaper than Map.set + Set.add combined. Also simplify the closure-cached eventList/links getters: eventList is now a plain property, links iterates the array directly. Update isEventGroup to discriminate on 'eventList' instead of 'events'. This was the key invariant that tests depended on: isEventGroup returning false for every group caused eventOrGroupIsFailureOrTimedOut, eventOrGroupIsCanceled, and eventOrGroupIsTerminated to silently fall through to the raw-event path (which always returned false for group objects). Test fixtures that built mock EventGroups with an events: [] array were also updated to use eventList: [] to match the new shape.
…tEvent buildGroupIndex: one pass over filteredGroups builds a Map<eventId, EventGroup> so any event→group lookup is O(1) instead of O(groups × events-per-group). Updated every call site that previously did groups.find(g => g.eventList.some(...)) to use the index: history-graph, event-summary-table, workflow-history-event, isMiddleEvent, and getGroupForEventOrPendingEvent all accept Map | EventGroup[] and fast-path on instanceof Map. getLastEvent: removed the ID-scanning loop (which compared numeric event IDs to find the maximum) and replaced it with eventList[eventList.length - 1]. Events are always appended in ascending-ID order by addToExistingGroup so the last element is always the most recent event. Eager classification fields: isFailureOrTimedOut, isCanceled, isTerminated, billableActions, and links were all getters that scanned or reduced over eventList on every access. They are now plain fields initialized from the first event in createGroupFor and updated by a new addEventToGroup helper called on each push in addToExistingGroup. Read cost drops from O(events- per-group) to a single field access, which matters because these are read from the SVG render path for every visible row.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| @@ -28,9 +29,9 @@ | |||
| <PendingNexusOperationCard operation={group.pendingNexusOperation} /> | |||
| {/if} | |||
Contributor
There was a problem hiding this comment.
⚠️ 'group' is possibly 'undefined'.
| return group?.eventIds?.has(hoveredEventId); | ||
| return group?.eventList?.some((e) => e.id === hoveredEventId); | ||
| }; | ||
|
|
Contributor
There was a problem hiding this comment.
⚠️ Parameter 'group' implicitly has an 'any' type.⚠️ Parameter 'hoveredEventId' implicitly has an 'any' type.
| return group?.eventList?.some((e) => e.id === hoveredEventId); | ||
| }; | ||
|
|
||
| onMount(async () => { |
Contributor
There was a problem hiding this comment.
⚠️ Parameter 'e' implicitly has an 'any' type.
| {@const group = getGroupForEventOrPendingEvent(allGroups, event)} | ||
| {@const group = getGroupForEventOrPendingEvent(groupIndex, event)} | ||
| <HistoryGraphRowVisual | ||
| {event} |
Contributor
There was a problem hiding this comment.
⚠️ Type 'EventGroup | undefined' is not assignable to type 'EventGroup'.
| _textWidthCache.set(cacheKey, width); | ||
| return width; | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
⚠️ Type 'SVGTextElement | undefined' is not assignable to type 'SVGTextElement'.
|
|
||
| expect(group.events.size).toBe(2); | ||
| expect(group.events.get(completedEvent.id)).toBe(completedEvent); | ||
| expect(group.eventList.length).toBe(2); |
Contributor
There was a problem hiding this comment.
⚠️ 'group' is possibly 'undefined'.
| @@ -140,8 +144,10 @@ describe('groupEvents', () => { | |||
|
|
|||
| const group = groups.find(({ id }) => id === scheduledEvent.id); | |||
|
|
|||
Contributor
There was a problem hiding this comment.
⚠️ 'group' is possibly 'undefined'.
|
|
||
| expect(group.events.size).toBe(2); | ||
| expect(group.events.get(completedEvent.id)).toBe(completedEvent); | ||
| expect(group.eventList.length).toBe(2); |
Contributor
There was a problem hiding this comment.
⚠️ 'group' is possibly 'undefined'.
| const id = getGroupId(event); | ||
| const group = createEventGroup(event); | ||
|
|
||
| const pa = isActivityTaskScheduledEvent(event) |
Contributor
There was a problem hiding this comment.
⚠️ 'event.activityTaskScheduledEventAttributes' is possibly 'null' or 'undefined'.⚠️ Argument of type 'string | null | undefined' is not assignable to parameter of type 'string'.
| import { fetchAllEventsBidirectional } from '$lib/services/events-service'; | ||
| import { | ||
| currentEventHistory, | ||
| fullEventHistory, |
Contributor
There was a problem hiding this comment.
⚠️ Module '"$lib/stores/events"' has no exported member 'loadedWorkflowKey'.
Contributor
|
| const initial = group.initialEvent; | ||
|
|
||
| if (isActivityTaskScheduledEvent(initial)) { | ||
| const pa = byActivityId.get( |
Contributor
There was a problem hiding this comment.
⚠️ Argument of type 'string | null | undefined' is not assignable to parameter of type 'string'.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description & motivation 💭
Screenshots (if applicable) 📸
Design Considerations 🎨
Testing 🧪
How was this tested 👻
Steps for others to test: 🚶🏽♂️🚶🏽♀️
Checklists
Draft Checklist
Merge Checklist
Issue(s) closed
Docs
Any docs updates needed?