Skip to content

Commit

Permalink
fix: Fix stack overflow for large # cells
Browse files Browse the repository at this point in the history
Previously, because of JS's lack of tail-call optimization, the layout
process can have a stack overflow due to recursion when the input
consists of more than ~2000 cells. This commit fixes that problem by
converting recursion to iteration.
  • Loading branch information
huy-nguyen committed May 8, 2018
1 parent 6a8c800 commit c686d58
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 32 deletions.
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,15 @@
"eslint": "4.19.1",
"eslint-plugin-jest": "21.15.1",
"eslint-plugin-react": "7.7.0",
"d3-random": "1.1.0",
"husky": "0.13.4",
"jest": "20.0.4",
"jest-junit": "3.7.0",
"rollup": "0.58.2",
"rollup-plugin-typescript2": "0.13.0",
"rollup-plugin-uglify": "3.0.0",
"semantic-release": "15.4.0",
"seedrandom": "2.4.3",
"ts-jest": "20.0.5",
"tslib": "1.7.1",
"tslint": "5.10.0",
Expand Down
19 changes: 19 additions & 0 deletions src/__tests__/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ import entry, {
squarify,
recurse,
} from '../index';
import seedRandom from 'seedrandom';
import {
randomLogNormal,
} from 'd3-random';

// Create an on-the-fly custom matcher for floating point comparison.
// Can be used anywhere in place of a number. Second argument is optional.
Expand Down Expand Up @@ -233,3 +237,18 @@ test('Package entry point', () => {
];
expect(actual).toEqual(expected);
});

test('Should not cause stack overflow for large input', () => {
const randomNumberGenerator = randomLogNormal.source(seedRandom('abc'))();
const size = 10000;
const data = [];
for (let i = 0; i < size; i += 1) {
const datum = {
value: Math.ceil(randomNumberGenerator() * 10000),
};
data.push(datum);
}


expect(() => entry(data, {x0: 0, y0: 0, x1: 1000, y1: 1000})).not.toThrow();
});
72 changes: 40 additions & 32 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,41 +227,49 @@ export const cutArea = (rect: IRect, area: number): IRect => {
};

export const squarify = <Custom>(
data: Array<INormalizedDatum<Custom>>,
currentRow: Array<INormalizedDatum<Custom>>,
rect: IRect,
stack: Array<ILayoutRect<Custom>>): Array<ILayoutRect<Custom>> => {

const dataLength = data.length;
if (dataLength === 0) {
const newCoordinates = getCoordinates(currentRow, rect);
const newStack: Array<ILayoutRect<Custom>> = stack.concat(newCoordinates);
return newStack;
}

const width = getShortestEdge(rect);
const nextDatum = data[0];
const restData = data.slice(1, dataLength);

if (doesAddingToRowImproveAspectRatio(currentRow, nextDatum, width)) {
const newRow = currentRow.concat(nextDatum);
return squarify(
restData, newRow, rect, stack,
);
} else {
const currentRowLength = currentRow.length;
let valueSum = 0;
for (let i = 0; i < currentRowLength; i += 1) {
valueSum += currentRow[i].normalizedValue;
inputData: Array<INormalizedDatum<Custom>>,
inputCurrentRow: Array<INormalizedDatum<Custom>>,
inputRect: IRect,
inputStack: Array<ILayoutRect<Custom>>): Array<ILayoutRect<Custom>> => {

let data: Array<INormalizedDatum<Custom>> = inputData,
currentRow: Array<INormalizedDatum<Custom>> = inputCurrentRow,
rect: IRect = inputRect,
stack: Array<ILayoutRect<Custom>> = inputStack;

while (true) {
const dataLength = data.length;
if (dataLength === 0) {
const newCoordinates = getCoordinates(currentRow, rect);
const newStack: Array<ILayoutRect<Custom>> = stack.concat(newCoordinates);
return newStack;
}

const newContainer = cutArea(rect, valueSum);
const newCoordinates = getCoordinates(currentRow, rect);
const newStack = stack.concat(newCoordinates);
const width = getShortestEdge(rect);
const nextDatum = data[0];
const restData = data.slice(1, dataLength);

if (doesAddingToRowImproveAspectRatio(currentRow, nextDatum, width)) {
const newRow = currentRow.concat(nextDatum);
data = restData;
currentRow = newRow;
rect = rect;
stack = stack;
} else {
const currentRowLength = currentRow.length;
let valueSum = 0;
for (let i = 0; i < currentRowLength; i += 1) {
valueSum += currentRow[i].normalizedValue;
}

return squarify(
data, [], newContainer, newStack,
);
const newContainer = cutArea(rect, valueSum);
const newCoordinates = getCoordinates(currentRow, rect);
const newStack = stack.concat(newCoordinates);
data = data;
currentRow = [];
rect = newContainer;
stack = newStack;
}
}
};

Expand Down
8 changes: 8 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1503,6 +1503,10 @@ cz-conventional-changelog@1.2.0:
right-pad "^1.0.1"
word-wrap "^1.0.3"

d3-random@1.1.0:
version "1.1.0"
resolved "https://registry.yarnpkg.com/d3-random/-/d3-random-1.1.0.tgz#6642e506c6fa3a648595d2b2469788a8d12529d3"

dashdash@^1.12.0:
version "1.14.1"
resolved "https://registry.yarnpkg.com/dashdash/-/dashdash-1.14.1.tgz#853cfa0f7cbe2fed5de20326b8dd581035f6e2f0"
Expand Down Expand Up @@ -4568,6 +4572,10 @@ sax@^1.2.1:
version "1.2.2"
resolved "https://registry.yarnpkg.com/sax/-/sax-1.2.2.tgz#fd8631a23bc7826bef5d871bdb87378c95647828"

seedrandom@2.4.3:
version "2.4.3"
resolved "https://registry.yarnpkg.com/seedrandom/-/seedrandom-2.4.3.tgz#2438504dad33917314bff18ac4d794f16d6aaecc"

semantic-release@15.4.0:
version "15.4.0"
resolved "https://registry.yarnpkg.com/semantic-release/-/semantic-release-15.4.0.tgz#4254cb5e93ce1ae6e6327f14aee377318823b5b8"
Expand Down

0 comments on commit c686d58

Please sign in to comment.