[fix] Maximum call stack size exceeded (#4694)

[milahu]

draft

start fixing #4694

// problem
[].push(...Array.from({ length: 1000*1000 })) // ... = spread operator
// Uncaught RangeError: Maximum call stack size exceeded

// solution
function pushArray(thisArray: any[], ...otherArrayList: any[]) {
  let count = 0;
  for (let a = 0; a < otherArrayList.length; a++) {
    const otherArray = otherArrayList[a];
    for (let i = 0; i < otherArray.length; i++) {
      thisArray.push(otherArray[i]);
    }
    count += otherArray.length;
  }
  return count;
};

pushArray(someArray, otherArray1, otherArray2, [otherItem])

the patch was generated with https://github.com/milahu/random/tree/master/svelte/patch-svelte-compiler-sources

ideally, the patch script would be implemented in eslint or putout

Before submitting the PR, please make sure you do the following

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• Prefix your PR title with [feat], [fix], [chore], or [docs].
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests

• Run the tests with npm test and lint the project with npm run lint

alternatives to pushArray

a very simple benchmark suggests that concat is faster than push
but performance depends on input data
todo: compare different methods with real-world data

to use dst = dst.concat(src), we must transform const dst to let dst

[milahu]

a test

svelte/test/limits/index.ts

import { svelte } from '../helpers';

// [].push(...Array.from({ length: 125*1000 }))
// Uncaught RangeError: Maximum call stack size exceeded

const critical_width = 125*1000; // 4 seconds
//const critical_width = 500*1000; // 12 seconds
//const critical_width = 1000*1000; // 32 seconds

describe('limits', () => {

  it(`can handle width of ${critical_width} elements`, function() {
    this.timeout(5*1000);
    // FIXME timeout does not kill the test, test hangs forever
    let source = '';
    for (let i = 0; i < critical_width; i++) {
      source += `<br>`;
    }
    svelte.compile(source, {
      generate: false // make test much faster
    });
  });

  const depth_step = 200;
  it(`find maximum depth of elements`, function() {
    for (let critical_depth = 800; ; critical_depth += depth_step ) {
      let source = '';
      for (let i = 0; i < critical_depth; i++) {
        source += `<div>`;
      }
      for (let i = 0; i < critical_depth; i++) {
        source += `</div>`;
      }
      try {
        svelte.compile(source, {
          generate: false // make test much faster
        });
      }
      catch (error) {
        // error: maximum call stack size exceeded
        console.log(`      maximum depth is between ${critical_depth - depth_step} and ${critical_depth} elements`)
        break
      }
    }
  });

});

output:

  limits
    ✓ can handle width of 300000 elements (6735ms)
      maximum depth is between 3200 and 3400 elements
    ✓ find maximum depth of elements (1461ms)

when the element-depth is too large (elements are nested too deep),
svelte throws the error Maximum call stack size exceeded

[benmccann]

does this need to return count? the return value doesn't seem to ever be used

[milahu]

compatibility with Array.prototype.push, almost zero cost

[benmccann]

actually, do we even need this method? It looks like we could just call concat everywhere this is being called for the same effect

[milahu]

*usually* push is faster than concat. depends on the exact input data. i can add the concat version for benchmarks

[dummdidumm]

Since we are talking about the compiler step here (not runtime), do we really need to be that performance sensitive here if it doesn't make a difference of more than 0.1 ms?

[milahu]

benchmarks to the rescue ... in the worst case, this is hot code

[milahu]

closing for #7203 - thanks all : )