From 45a548c084c00853acfabed760f04c77fa78d43f Mon Sep 17 00:00:00 2001 From: brianlund-wandb Date: Thu, 12 Dec 2024 22:08:18 -0800 Subject: [PATCH 1/3] optimize finding next tick up or down --- .../components/elements/SliderInput.test.tsx | 63 +++++++++++++++++++ .../components/elements/SliderInput.tsx | 35 ++++++++--- 2 files changed, 91 insertions(+), 7 deletions(-) create mode 100644 weave-js/src/common/components/elements/SliderInput.test.tsx diff --git a/weave-js/src/common/components/elements/SliderInput.test.tsx b/weave-js/src/common/components/elements/SliderInput.test.tsx new file mode 100644 index 00000000000..ae002fb36ba --- /dev/null +++ b/weave-js/src/common/components/elements/SliderInput.test.tsx @@ -0,0 +1,63 @@ +import {getClosestTick} from './SliderInput'; + +describe('getClosestTick', () => { + test('lower previous value returns next greater', () => { + let ticks = [...new Set([2, 4, 6, 8, 10])], + previous = 4, + val = 5; + + expect(getClosestTick(ticks, val, previous)).toBe(6); + }); + test('greater previous value returns next lesser', () => { + let ticks = [...new Set([2, 4, 6, 8, 10])], + previous = 4, + val = 3; + const actual = getClosestTick(ticks, val, previous); + expect(actual).toBe(2); + }); + test('greater previous value returns next lesser, consecutive', () => { + let ticks = [...new Set([1, 2, 3, 4, 5, 6])], + previous = 4, + val = 3; + const actual = getClosestTick(ticks, val, previous); + expect(actual).toBe(3); + }); + test('lower previous value returns next greater, consecutive', () => { + let ticks = [...new Set([1, 2, 3, 4, 5, 6])], + previous = 3, + val = 4; + const actual = getClosestTick(ticks, val, previous); + expect(actual).toBe(4); + }); + test('lower previous value returns next greater, erratic', () => { + let ticks = [...new Set([1, 4, 5, 7, 9, 12])], + previous = 9, + val = 10; + const actual = getClosestTick(ticks, val, previous); + expect(actual).toBe(12); + }); + test('greater previous value returns next lower, erratic', () => { + let ticks = [...new Set([1, 2, 5, 7, 9, 12])], + previous = 5, + val = 4; + const actual = getClosestTick(ticks, val, previous); + expect(actual).toBe(2); + }); + + // Modified logic, retaining for expected performance + test('large number of ticks', () => { + let ticks = [...new Set()], + previous = 500000, + val = 500001; + + for (let i = 0; i < 10000000; i += 2) { + ticks.push(i); + } + const start = Date.now(); + const actual = getClosestTick(ticks, val, previous); + const end = Date.now(); + const duration = end - start; + expect(actual).toBe(500002); + expect(duration).toBeLessThanOrEqual(1); + }); +}); diff --git a/weave-js/src/common/components/elements/SliderInput.tsx b/weave-js/src/common/components/elements/SliderInput.tsx index dce75615c93..77193977128 100644 --- a/weave-js/src/common/components/elements/SliderInput.tsx +++ b/weave-js/src/common/components/elements/SliderInput.tsx @@ -78,7 +78,7 @@ const SliderInput: React.FC = React.memo( newVal = min; } if (ticks != null) { - newVal = getClosestTick(ticks, newVal); + newVal = getClosestTick(ticks, newVal, sliderValue); } setSliderValue(newVal); onChangeDebounced(newVal); @@ -172,17 +172,38 @@ const SliderInput: React.FC = React.memo( export default SliderInput; -function getClosestTick(ticks: number[], val: number): number { +export function getClosestTick( + ticks: number[], + val: number, + prev: number +): number { let closest = val; + const increasing = val > prev; let minDiff = Number.MAX_VALUE; - for (const tick of ticks) { + // Binary search for the closest tick + let left = 0; + let right = ticks.length - 1; + + while (left <= right) { + const mid = Math.floor((left + right) / 2); + const tick = ticks[mid]; const diff = Math.abs(tick - val); - if (diff >= minDiff) { - break; + + // Only update closest if the tick is in the right direction + if ( + diff < minDiff && + ((increasing && tick >= val) || (!increasing && tick <= val)) + ) { + closest = tick; + minDiff = diff; + } + + if (tick < val) { + left = mid + 1; + } else { + right = mid - 1; } - closest = tick; - minDiff = diff; } return closest; From 2a6fcff70fb1ad7dfe46681275f25821d4102622 Mon Sep 17 00:00:00 2001 From: brianlund-wandb Date: Thu, 12 Dec 2024 23:08:47 -0800 Subject: [PATCH 2/3] addressing static analysic error --- weave-js/src/common/components/elements/SliderInput.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/weave-js/src/common/components/elements/SliderInput.tsx b/weave-js/src/common/components/elements/SliderInput.tsx index 77193977128..1afbb8f064b 100644 --- a/weave-js/src/common/components/elements/SliderInput.tsx +++ b/weave-js/src/common/components/elements/SliderInput.tsx @@ -83,7 +83,7 @@ const SliderInput: React.FC = React.memo( setSliderValue(newVal); onChangeDebounced(newVal); }, - [ticks, min, max, allowGreaterThanMax, onChangeDebounced] + [ticks, min, max, allowGreaterThanMax, sliderValue, onChangeDebounced] ); React.useEffect(() => { From 01b7661d2e8063a38b234a49b6cddffbc1df2b91 Mon Sep 17 00:00:00 2001 From: brianlund-wandb Date: Fri, 13 Dec 2024 11:34:13 -0800 Subject: [PATCH 3/3] tslint issues --- .../components/elements/SliderInput.test.tsx | 51 ++++++++++--------- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/weave-js/src/common/components/elements/SliderInput.test.tsx b/weave-js/src/common/components/elements/SliderInput.test.tsx index ae002fb36ba..a712fefc354 100644 --- a/weave-js/src/common/components/elements/SliderInput.test.tsx +++ b/weave-js/src/common/components/elements/SliderInput.test.tsx @@ -2,57 +2,62 @@ import {getClosestTick} from './SliderInput'; describe('getClosestTick', () => { test('lower previous value returns next greater', () => { - let ticks = [...new Set([2, 4, 6, 8, 10])], - previous = 4, - val = 5; - + const ticks = [2, 4, 6, 8, 10]; + const previous = 4; + const val = 5; expect(getClosestTick(ticks, val, previous)).toBe(6); }); + test('lower previous value returns next greater, large step', () => { + const ticks = [2, 4, 60, 80, 10]; + const previous = 4; + const val = 5; + expect(getClosestTick(ticks, val, previous)).toBe(60); + }); test('greater previous value returns next lesser', () => { - let ticks = [...new Set([2, 4, 6, 8, 10])], - previous = 4, - val = 3; + const ticks = [2, 4, 6, 8, 10]; + const previous = 4; + const val = 3; const actual = getClosestTick(ticks, val, previous); expect(actual).toBe(2); }); test('greater previous value returns next lesser, consecutive', () => { - let ticks = [...new Set([1, 2, 3, 4, 5, 6])], - previous = 4, - val = 3; + const ticks = [1, 2, 3, 4, 5, 6]; + const previous = 4; + const val = 3; const actual = getClosestTick(ticks, val, previous); expect(actual).toBe(3); }); test('lower previous value returns next greater, consecutive', () => { - let ticks = [...new Set([1, 2, 3, 4, 5, 6])], - previous = 3, - val = 4; + const ticks = [1, 2, 3, 4, 5, 6]; + const previous = 3; + const val = 4; const actual = getClosestTick(ticks, val, previous); expect(actual).toBe(4); }); test('lower previous value returns next greater, erratic', () => { - let ticks = [...new Set([1, 4, 5, 7, 9, 12])], - previous = 9, - val = 10; + const ticks = [1, 4, 5, 7, 9, 12]; + const previous = 9; + const val = 10; const actual = getClosestTick(ticks, val, previous); expect(actual).toBe(12); }); test('greater previous value returns next lower, erratic', () => { - let ticks = [...new Set([1, 2, 5, 7, 9, 12])], - previous = 5, - val = 4; + const ticks = [1, 2, 5, 7, 9, 12]; + const previous = 5; + const val = 4; const actual = getClosestTick(ticks, val, previous); expect(actual).toBe(2); }); // Modified logic, retaining for expected performance test('large number of ticks', () => { - let ticks = [...new Set()], - previous = 500000, - val = 500001; - + const ticks = []; for (let i = 0; i < 10000000; i += 2) { ticks.push(i); } + + const previous = 500000; + const val = 500001; const start = Date.now(); const actual = getClosestTick(ticks, val, previous); const end = Date.now();