mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-01-26 19:09:52 +00:00
performance fix: prevent gcd infinite loop with floating-point step values (#7258)
## Summary report and fix https://github.com/Comfy-Org/ComfyUI_frontend/issues/3919 - Convert recursive gcd to iterative to avoid stack overflow - Add epsilon tolerance (1e-10) for floating-point precision issues This fixes workflow loading hangs when node trying merge values like 0.01 and 0.001, which caused the original recursive gcd to run indefinitely due to floating-point modulo never reaching exactly zero. please notice, we need both iterative and epsilon together to fix this gcd issue Call Chain PrimitiveNode.onAfterGraphConfigured → #mergeWidgetConfig → #isValidConnection → mergeIfValid → mergeInputSpec → mergeNumericInputSpec → lcm(step1, step2) → gcd(a, b) ← Problem here Why It Happened When some nodes connect to multiple nodes, it may merge values using LCM, which internally calls GCD. Original recursive implementation: ``` export const gcd = (a: number, b: number): number => { return b === 0 ? a : gcd(b, a % b) } ``` Issues: 1. Stack Overflow: Recursive calls with many nodes exhausted the call stack. 2. Floating-Point Precision: For values like gcd(0.01, 0.001): ` 0.01 % 0.001 = 0.0009999999999999994 // Not exactly 0!` 3. Due to Ifloating-point representation, the modulo never reaches exactly zero, causing hundreds or thousands of iterations. ## Screenshots ### before https://github.com/user-attachments/assets/cca4342c-a882-4590-a8d4-1e0bea19e5b7 ### fix with only iterative, without epsilon https://github.com/user-attachments/assets/1dc52aa4-a86a-40b5-8bac-904094c4c36b ### final fix with iterative and epsilon https://github.com/user-attachments/assets/7b868b50-c3c9-4be4-8594-27cecbc08a26 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7258-performance-fix-prevent-gcd-infinite-loop-with-floating-point-step-values-2c46d73d3650818cbe8cf455c934a114) by [Unito](https://www.unito.io)
This commit is contained in:
@@ -2,14 +2,38 @@ import type { ReadOnlyRect } from '@/lib/litegraph/src/interfaces'
|
||||
import type { Bounds } from '@/renderer/core/layout/types'
|
||||
|
||||
/**
|
||||
* Finds the greatest common divisor (GCD) for two numbers.
|
||||
* Finds the greatest common divisor (GCD) for two numbers using iterative
|
||||
* Euclidean algorithm. Uses iteration instead of recursion to avoid stack
|
||||
* overflow with large inputs or small floating-point step values.
|
||||
*
|
||||
* For floating-point numbers, uses a tolerance-based approach to handle
|
||||
* precision issues and limits iterations to prevent hangs.
|
||||
*
|
||||
* @param a - The first number.
|
||||
* @param b - The second number.
|
||||
* @returns The GCD of the two numbers.
|
||||
*/
|
||||
export const gcd = (a: number, b: number): number => {
|
||||
return b === 0 ? a : gcd(b, a % b)
|
||||
// Use absolute values to handle negative numbers
|
||||
let x = Math.abs(a)
|
||||
let y = Math.abs(b)
|
||||
|
||||
// Handle edge cases
|
||||
if (x === 0) return y
|
||||
if (y === 0) return x
|
||||
|
||||
// For floating-point numbers, use tolerance-based comparison
|
||||
// This prevents infinite loops due to floating-point precision issues
|
||||
const epsilon = 1e-10
|
||||
const maxIterations = 100
|
||||
|
||||
let iterations = 0
|
||||
while (y > epsilon && iterations < maxIterations) {
|
||||
;[x, y] = [y, x % y]
|
||||
iterations++
|
||||
}
|
||||
|
||||
return x
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -12,6 +12,19 @@ describe('mathUtil', () => {
|
||||
expect(gcd(0, 5)).toBe(5)
|
||||
expect(gcd(5, 0)).toBe(5)
|
||||
})
|
||||
|
||||
it('should handle negative numbers', () => {
|
||||
expect(gcd(-48, 18)).toBe(6)
|
||||
expect(gcd(48, -18)).toBe(6)
|
||||
expect(gcd(-48, -18)).toBe(6)
|
||||
})
|
||||
|
||||
it('should not cause stack overflow with small floating-point step values', () => {
|
||||
// This would cause Maximum call stack size exceeded with recursive impl
|
||||
// when used in lcm calculations with small step values
|
||||
expect(() => gcd(0.0001, 0.0003)).not.toThrow()
|
||||
expect(() => gcd(1e-10, 1e-9)).not.toThrow()
|
||||
})
|
||||
})
|
||||
|
||||
describe('lcm', () => {
|
||||
|
||||
Reference in New Issue
Block a user