Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Maximum call stack size exceeded with array.push #302

Conversation

shardAstronaut
Copy link
Contributor

Description

When indexing contracts with a big number of events the array.push method will throw an error.
When you use spread operator all items of the source array are stored in the stack as arguments list, so having a large number of items (~ > 100K) will cause the this stack size exceed. The most simple work-around is to manually push all items one by one.
https://stackoverflow.com/questions/61740599/rangeerror-maximum-call-stack-size-exceeded-with-array-push

@shardAstronaut shardAstronaut changed the title fix: Maximum call stack size exceeded with array.push (Bug: last_inde… fix: Maximum call stack size exceeded with array.push Jun 26, 2024
@bonustrack bonustrack requested a review from Sekhmet June 26, 2024 15:28
Copy link
Contributor

@Sekhmet Sekhmet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know if this issue still happens in your case if we use concat instead of push in a loop?

I wonder if concat:

  1. solves the issue
  2. is faster

@shardAstronaut
Copy link
Contributor Author

shardAstronaut commented Jun 26, 2024

Do you know if this issue still happens in your case if we use concat instead of push in a loop?

I wonder if concat:

  1. solves the issue
  2. is faster
  1. Yes it solves the issue
  2. Looks like concat is faster than push based on this discussion https://stackoverflow.com/questions/48865710/spread-operator-vs-array-concat.
    I can update the PR to use concat it's cleaner, wdyt?

@Sekhmet
Copy link
Contributor

Sekhmet commented Jun 27, 2024

Let's do concat then, thanks!

…xed_block reset to Zero after reaching the seed blocks checkpoint-labs#300)

When indexing contracts with a big number of events the array.push method will throw an error. When you use spread operator all items of the source array are stored in the stack as arguments list, so having a large number of items (~ > 100K) will cause the this stack size exceed. The most simple work-around is to manually push all items one by one.
@shardAstronaut shardAstronaut force-pushed the fix/array-push-exceeded-stack-call branch from 796309c to 95fcfca Compare June 27, 2024 09:17
@shardAstronaut shardAstronaut requested a review from Sekhmet June 27, 2024 09:17
@shardAstronaut
Copy link
Contributor Author

Do you know what changed in Starknet sequancer to start getting this error now? I was indexing the ETH contract for more than 4 months and never get this error.

@Sekhmet
Copy link
Contributor

Sekhmet commented Jun 27, 2024

Do you know what changed in Starknet sequancer to start getting this error now? I was indexing the ETH contract for more than 4 months and never get this error.

Most likely it's caused by the fact that we changed the way we fetch events to find blocks we need to index (before we fetched all events, but only for 100 blocks at the time, now we fetch 1000 blocks or more at the time, but filtering events that only come from contracts we track). Generally it causes way faster indexing, but looks like there is room for improvements for certain contracts.

Copy link
Contributor

@Sekhmet Sekhmet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@Sekhmet
Copy link
Contributor

Sekhmet commented Jun 27, 2024

Thank you!

@Sekhmet Sekhmet merged commit 65ea05e into checkpoint-labs:master Jun 27, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants