From 8b6ef7b325feca294cf9c8f526bb4958f28a7a03 Mon Sep 17 00:00:00 2001
From: lelemm <lelemm@gmail.com>
Date: Tue, 18 Jun 2024 14:39:24 -0300
Subject: [PATCH] Removed recursion in place for iterable solution to prevent
stack overflow (#2848)
---
.../src/components/settings/Experimental.tsx | 3 +
.../src/hooks/useFeatureFlag.ts | 1 +
.../spreadsheet/graph-data-structure.ts | 96 ++++++++++++++++---
packages/loot-core/src/types/prefs.d.ts | 3 +-
upcoming-release-notes/2848.md | 6 ++
5 files changed, 95 insertions(+), 14 deletions(-)
create mode 100644 upcoming-release-notes/2848.md
diff --git a/packages/desktop-client/src/components/settings/Experimental.tsx b/packages/desktop-client/src/components/settings/Experimental.tsx
index 41fe8d6e9..4b809341f 100644
--- a/packages/desktop-client/src/components/settings/Experimental.tsx
+++ b/packages/desktop-client/src/components/settings/Experimental.tsx
@@ -90,6 +90,9 @@ export function ExperimentalFeatures() {
Goal templates
</FeatureToggle>
<FeatureToggle flag="simpleFinSync">SimpleFIN sync</FeatureToggle>
+ <FeatureToggle flag="iterableTopologicalSort">
+ Iterable topological sort budget
+ </FeatureToggle>
</View>
) : (
<Link
diff --git a/packages/desktop-client/src/hooks/useFeatureFlag.ts b/packages/desktop-client/src/hooks/useFeatureFlag.ts
index 010b172ec..0089686be 100644
--- a/packages/desktop-client/src/hooks/useFeatureFlag.ts
+++ b/packages/desktop-client/src/hooks/useFeatureFlag.ts
@@ -9,6 +9,7 @@ const DEFAULT_FEATURE_FLAG_STATE: Record<FeatureFlag, boolean> = {
customReports: false,
spendingReport: false,
simpleFinSync: false,
+ iterableTopologicalSort: true,
};
export function useFeatureFlag(name: FeatureFlag): boolean {
diff --git a/packages/loot-core/src/server/spreadsheet/graph-data-structure.ts b/packages/loot-core/src/server/spreadsheet/graph-data-structure.ts
index 76fe24762..be2669d1f 100644
--- a/packages/loot-core/src/server/spreadsheet/graph-data-structure.ts
+++ b/packages/loot-core/src/server/spreadsheet/graph-data-structure.ts
@@ -1,3 +1,5 @@
+import { getPrefs } from '../prefs';
+
// @ts-strict-ignore
export function Graph() {
const graph = {
@@ -76,14 +78,38 @@ export function Graph() {
return graph;
}
- function topologicalSortUntil(name, visited, sorted) {
+ function topologicalSort(sourceNodes) {
+ const visited = new Set();
+ const sorted = [];
+ const prefs = getPrefs();
+ const iterableTopologicalSort =
+ prefs != null ? prefs['flags.iterableTopologicalSort'] : false;
+
+ sourceNodes.forEach(name => {
+ if (!visited.has(name)) {
+ if (iterableTopologicalSort) {
+ topologicalSortIterable(name, visited, sorted);
+ } else {
+ topologicalSortUntil(name, visited, sorted, 0);
+ }
+ }
+ });
+
+ return sorted;
+ }
+
+ function topologicalSortUntil(name, visited, sorted, level) {
visited.add(name);
+ if (level > 2500) {
+ console.error('Limit of recursions reached while sorting budget: 2500');
+ return;
+ }
const iter = adjacent(name).values();
let cur = iter.next();
while (!cur.done) {
if (!visited.has(cur.value)) {
- topologicalSortUntil(cur.value, visited, sorted);
+ topologicalSortUntil(cur.value, visited, sorted, level + 1);
}
cur = iter.next();
}
@@ -91,17 +117,54 @@ export function Graph() {
sorted.unshift(name);
}
- function topologicalSort(sourceNodes) {
- const visited = new Set();
- const sorted = [];
+ function topologicalSortIterable(name, visited, sorted) {
+ const stackTrace: StackItem[] = [];
- sourceNodes.forEach(name => {
- if (!visited.has(name)) {
- topologicalSortUntil(name, visited, sorted);
- }
+ stackTrace.push({
+ count: -1,
+ value: name,
+ parent: '',
+ level: 0,
});
- return sorted;
+ while (stackTrace.length > 0) {
+ const current = stackTrace.slice(-1)[0];
+
+ const adjacents = adjacent(current.value);
+ if (current.count === -1) {
+ current.count = adjacents.size;
+ }
+
+ if (current.count > 0) {
+ const iter = adjacents.values();
+ let cur = iter.next();
+ while (!cur.done) {
+ if (!visited.has(cur.value)) {
+ stackTrace.push({
+ count: -1,
+ parent: current.value,
+ value: cur.value,
+ level: current.level + 1,
+ });
+ } else {
+ current.count--;
+ }
+ cur = iter.next();
+ }
+ } else {
+ if (!visited.has(current.value)) {
+ visited.add(current.value);
+ sorted.unshift(current.value);
+ }
+
+ const removed = stackTrace.pop();
+ for (let i = 0; i < stackTrace.length; i++) {
+ if (stackTrace[i].value === removed.parent) {
+ stackTrace[i].count--;
+ }
+ }
+ }
+ }
}
function generateDOT() {
@@ -113,11 +176,18 @@ export function Graph() {
});
return `
- digraph G {
- ${edgeStrings.join('\n').replace(/!/g, '_')}
- }
+ digraph G {
+ ${edgeStrings.join('\n').replace(/!/g, '_')}
+ }
`;
}
return graph;
}
+
+interface StackItem {
+ count: number;
+ value: string;
+ parent: string;
+ level: number;
+}
diff --git a/packages/loot-core/src/types/prefs.d.ts b/packages/loot-core/src/types/prefs.d.ts
index 161c85faa..38f7b1a97 100644
--- a/packages/loot-core/src/types/prefs.d.ts
+++ b/packages/loot-core/src/types/prefs.d.ts
@@ -5,7 +5,8 @@ export type FeatureFlag =
| 'goalTemplatesEnabled'
| 'customReports'
| 'spendingReport'
- | 'simpleFinSync';
+ | 'simpleFinSync'
+ | 'iterableTopologicalSort';
export type LocalPrefs = Partial<
{
diff --git a/upcoming-release-notes/2848.md b/upcoming-release-notes/2848.md
new file mode 100644
index 000000000..35fc24299
--- /dev/null
+++ b/upcoming-release-notes/2848.md
@@ -0,0 +1,6 @@
+---
+category: Bugfix
+authors: [lelemm]
+---
+
+Remove recursion from topological sort to prevent stack overflow
--
GitLab