mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-20 06:20:11 +00:00
fix: reduce minimap per-frame allocations and O(N) link serialization
- Replace JSON.stringify(g.links) with O(1) version+size check - Reuse hoisted Set for node ID cleanup instead of new Set+map per frame - Cache MinimapDataSourceFactory instance, invalidate on graph change - Clean up all cached state in destroy()
This commit is contained in:
@@ -37,8 +37,11 @@ describe('useMinimapGraph', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
|
||||
const linksMap = new Map([[1, createMockLLink({ id: 1 })]])
|
||||
mockGraph = createMockLGraph({
|
||||
id: 'test-graph-123',
|
||||
_version: 0,
|
||||
_links: linksMap,
|
||||
_nodes: [
|
||||
createMockLGraphNode({ id: '1', pos: [100, 100], size: [150, 80] }),
|
||||
createMockLGraphNode({ id: '2', pos: [300, 200], size: [120, 60] })
|
||||
@@ -199,24 +202,53 @@ describe('useMinimapGraph', () => {
|
||||
expect(graphManager.updateFlags.value.nodes).toBe(true)
|
||||
})
|
||||
|
||||
it('should detect connection changes', () => {
|
||||
it('should detect connection changes via graph version', () => {
|
||||
const graphRef = ref(mockGraph) as Ref<LGraph | null>
|
||||
const graphManager = useMinimapGraph(graphRef, onGraphChangedMock)
|
||||
|
||||
// Cache initial state
|
||||
graphManager.checkForChanges()
|
||||
|
||||
// Change connections
|
||||
mockGraph.links = createMockLinks([
|
||||
createMockLLink({ id: 1 }),
|
||||
createMockLLink({ id: 2 })
|
||||
])
|
||||
// Increment graph version (simulates add/remove node/link)
|
||||
mockGraph._version++
|
||||
|
||||
const hasChanges = graphManager.checkForChanges()
|
||||
expect(hasChanges).toBe(true)
|
||||
expect(graphManager.updateFlags.value.connections).toBe(true)
|
||||
})
|
||||
|
||||
it('should detect connection changes via link count', () => {
|
||||
const graphRef = ref(mockGraph) as Ref<LGraph | null>
|
||||
const graphManager = useMinimapGraph(graphRef, onGraphChangedMock)
|
||||
|
||||
// Cache initial state
|
||||
graphManager.checkForChanges()
|
||||
|
||||
// Add a link to the backing Map
|
||||
const newLink = createMockLLink({ id: 2 })
|
||||
mockGraph._links.set(2, newLink)
|
||||
|
||||
const hasChanges = graphManager.checkForChanges()
|
||||
expect(hasChanges).toBe(true)
|
||||
expect(graphManager.updateFlags.value.connections).toBe(true)
|
||||
})
|
||||
|
||||
it('should not detect connection changes when version and count unchanged', () => {
|
||||
const graphRef = ref(mockGraph) as Ref<LGraph | null>
|
||||
const graphManager = useMinimapGraph(graphRef, onGraphChangedMock)
|
||||
|
||||
// Cache initial state
|
||||
graphManager.checkForChanges()
|
||||
|
||||
// Reset flags
|
||||
graphManager.updateFlags.value.connections = false
|
||||
|
||||
// No changes to version or link count
|
||||
const hasChanges = graphManager.checkForChanges()
|
||||
expect(hasChanges).toBe(false)
|
||||
expect(graphManager.updateFlags.value.connections).toBe(false)
|
||||
})
|
||||
|
||||
it('should handle node removal in callbacks', () => {
|
||||
const originalOnNodeRemoved = vi.fn()
|
||||
mockGraph.onNodeRemoved = originalOnNodeRemoved
|
||||
|
||||
@@ -12,7 +12,7 @@ import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
|
||||
import { api } from '@/scripts/api'
|
||||
|
||||
import { MinimapDataSourceFactory } from '../data/MinimapDataSourceFactory'
|
||||
import type { UpdateFlags } from '../types'
|
||||
import type { IMinimapDataSource, UpdateFlags } from '../types'
|
||||
|
||||
interface GraphCallbacks {
|
||||
onNodeAdded?: (node: LGraphNode) => void
|
||||
@@ -26,7 +26,8 @@ export function useMinimapGraph(
|
||||
onGraphChanged: () => void
|
||||
) {
|
||||
const nodeStatesCache = new Map<NodeId, string>()
|
||||
const linksCache = ref<string>('')
|
||||
let lastLinkCount = 0
|
||||
let lastGraphVersion = -1
|
||||
const lastNodeCount = ref(0)
|
||||
const updateFlags = ref<UpdateFlags>({
|
||||
bounds: false,
|
||||
@@ -35,6 +36,13 @@ export function useMinimapGraph(
|
||||
viewport: false
|
||||
})
|
||||
|
||||
// Reusable Set for node ID cleanup (avoids allocation per frame)
|
||||
const currentNodeIds = new Set<NodeId>()
|
||||
|
||||
// Cached data source instance — invalidated when graph changes
|
||||
let cachedDataSource: IMinimapDataSource | null = null
|
||||
let cachedDataSourceGraph: LGraph | null = null
|
||||
|
||||
// Track LayoutStore version for change detection
|
||||
const layoutStoreVersion = layoutStore.getVersion()
|
||||
|
||||
@@ -114,6 +122,15 @@ export function useMinimapGraph(
|
||||
originalCallbacksMap.delete(g.id)
|
||||
}
|
||||
|
||||
function getDataSource(g: LGraph): IMinimapDataSource {
|
||||
if (cachedDataSource && cachedDataSourceGraph === g) {
|
||||
return cachedDataSource
|
||||
}
|
||||
cachedDataSource = MinimapDataSourceFactory.create(g)
|
||||
cachedDataSourceGraph = g
|
||||
return cachedDataSource
|
||||
}
|
||||
|
||||
const checkForChangesInternal = () => {
|
||||
const g = graph.value
|
||||
if (!g) return false
|
||||
@@ -122,8 +139,7 @@ export function useMinimapGraph(
|
||||
let positionChanged = false
|
||||
let connectionChanged = false
|
||||
|
||||
// Use unified data source for change detection
|
||||
const dataSource = MinimapDataSourceFactory.create(g)
|
||||
const dataSource = getDataSource(g)
|
||||
|
||||
// Check for node count changes
|
||||
const currentNodeCount = dataSource.getNodeCount()
|
||||
@@ -144,8 +160,11 @@ export function useMinimapGraph(
|
||||
}
|
||||
}
|
||||
|
||||
// Clean up removed nodes from cache
|
||||
const currentNodeIds = new Set(nodes.map((n) => n.id))
|
||||
// Clean up removed nodes from cache (reuse Set to avoid allocation)
|
||||
currentNodeIds.clear()
|
||||
for (const node of nodes) {
|
||||
currentNodeIds.add(node.id)
|
||||
}
|
||||
for (const [nodeId] of nodeStatesCache) {
|
||||
if (!currentNodeIds.has(nodeId)) {
|
||||
nodeStatesCache.delete(nodeId)
|
||||
@@ -153,11 +172,17 @@ export function useMinimapGraph(
|
||||
}
|
||||
}
|
||||
|
||||
// TODO: update when Layoutstore tracks links
|
||||
const currentLinks = JSON.stringify(g.links || {})
|
||||
if (currentLinks !== linksCache.value) {
|
||||
// Detect connection changes via graph version + link count
|
||||
// (avoids JSON.stringify of all links every frame)
|
||||
const currentVersion = g._version
|
||||
const currentLinkCount = g._links?.size ?? 0
|
||||
if (
|
||||
currentVersion !== lastGraphVersion ||
|
||||
currentLinkCount !== lastLinkCount
|
||||
) {
|
||||
connectionChanged = true
|
||||
linksCache.value = currentLinks
|
||||
lastGraphVersion = currentVersion
|
||||
lastLinkCount = currentLinkCount
|
||||
}
|
||||
|
||||
if (structureChanged || positionChanged) {
|
||||
@@ -184,13 +209,17 @@ export function useMinimapGraph(
|
||||
const destroy = () => {
|
||||
cleanupEventListeners()
|
||||
api.removeEventListener('graphChanged', handleGraphChangedThrottled)
|
||||
nodeStatesCache.clear()
|
||||
clearCache()
|
||||
}
|
||||
|
||||
const clearCache = () => {
|
||||
nodeStatesCache.clear()
|
||||
linksCache.value = ''
|
||||
currentNodeIds.clear()
|
||||
lastLinkCount = 0
|
||||
lastGraphVersion = -1
|
||||
lastNodeCount.value = 0
|
||||
cachedDataSource = null
|
||||
cachedDataSourceGraph = null
|
||||
}
|
||||
|
||||
return {
|
||||
|
||||
Reference in New Issue
Block a user