automerge-classic icon indicating copy to clipboard operation
automerge-classic copied to clipboard

Maximum call stack exceeded when initializing Automerge.Text with large (500kb) string

Open cgauld opened this issue 2 years ago • 4 comments

Hi!

We're using Preview 6 and have found that initializing Automerge.Text, like this:

doc.text = new Automerge.Text(largeString);

Causes the following error stack when called with a ~500kb string.

Uncaught (in promise) RangeError: Maximum call stack size exceeded
    at updateTextObject (...)
    at interpretPatch (...)
    at getValue (...)
    at applyProperties (...)
    at updateMapObject (...)
    at Context.interpretPatch [as applyPatch] (...)
    at Context.applyAtPath (...)
    at Context.setMapKey (...)
    at Object.set (...)

I haven't spotted an issue for this so I thought I'd add one. Would be grateful to hear of any thoughts/workarounds.

Thanks all!

cgauld avatar Jun 30 '22 12:06 cgauld

Oh, this is unfortunate. It's probably due to the code using array.splice(index, 0, ...elements) to insert a large number of elements all at once, and JavaScript doesn't like the spread syntax resulting in a method call with hundreds of thousands of arguments.

The quickest workaround for now is probably to insert the text in several chunks, spread across several changes. Something like this:

let doc = Automerge.change(Automerge.init(), doc => doc.text = new Automerge.Text())

for (let chunk of largeString.match(/(.{1,10000})/g)) {
  doc = Automerge.change(doc, doc => doc.text.insertAt(doc.text.length, ...chunk))
}

Not tested, but please give it a try and let me know if it helps!

ept avatar Jul 01 '22 15:07 ept

Interesting. I'm also getting this error, when trying to add a rather large object. I'll see how many properties I'd need to remove before I don't get this error and will report back.

michaelpalumbo avatar Aug 06 '22 02:08 michaelpalumbo

I did some testing since I happened to run into this issue with some non-automerge code. In Firefox console the limit seems to be exactly 500k arguments:

(function () {})(...Array(500_000).fill(null))
// result: undefined

(function () {})(...Array(500_001).fill(null))
// result: Uncaught RangeError: too many function arguments

In Chrome console around 125k arguments:

(function () {})(...Array(124_956).fill(null))
// result: undefined

(function () {})(...Array(124_957).fill(null))
// result: Uncaught RangeError: Maximum call stack size exceeded

In Node.js REPL around 120k arguments:

(function () {})(...Array(120_425).fill(null))
// result: undefined

(function () {})(...Array(120_426).fill(null))
// result: RangeError: Maximum call stack size exceeded

Obviously there could be some other things that affect the limit at least in V8 (previous call stack depth?), but there's some ballpark figures.

fo-fo avatar Aug 17 '22 16:08 fo-fo

Thanks for those experiments. Looks like the best way of fixing this would be to change the API so that you pass in an array, rather than a variable number of arguments, to insert multiple elements at once. Then we wouldn't be subject to this stack size limitation. However, if we do change the API, it would be best if we could figure out some way of avoiding making it a breaking change. Also, JavaScript's Array.splice() and Array.push() rely on passing multiple arguments; changing them to take a single array argument would make Automerge's API inconsistent with those methods. I'm not sure how best to resolve these conflicting concerns.

ept avatar Aug 18 '22 13:08 ept