Skip to content

fix: avoid stack overflow when loading bracket guides for large files#134189

Merged
hediet merged 1 commit intomicrosoft:mainfrom
yume-chan:patch-1
Sep 30, 2021
Merged

fix: avoid stack overflow when loading bracket guides for large files#134189
hediet merged 1 commit intomicrosoft:mainfrom
yume-chan:patch-1

Conversation

@yume-chan
Copy link
Contributor

This PR fixes #133923

a.push(...b) will push all items of b onto call stack, if b is too large, this can cause stack overflow.

However, loading bracket info for large files can still be terribly slow.

@hediet
Copy link
Member

hediet commented Sep 30, 2021

Argh, I was wondering why the stack overflow would happen there.
I guess that makes my rather stupid fix obsolete 😅 - Thank you!

I wonder why b is so large though. The amount of bracket pairs should be limited by the viewport/minimap size. I should investigate why the minimap is requesting these decorations anyway.

I think it is not good to request all decorations at any point, I'll have to look into that.

@hediet hediet merged commit f94d326 into microsoft:main Sep 30, 2021
@hediet hediet added this to the September 2021 milestone Sep 30, 2021
@hediet hediet self-assigned this Sep 30, 2021
@yume-chan
Copy link
Contributor Author

yume-chan commented Sep 30, 2021

I didn't dig that deep, now I added a log for b's length and for my file it's 130871.

My file is an iife and @alexdima's repro is with a large json file. Both files have a left bracket at start and corresponding right bracket at end. I wonder if it's not possible to find the right bracket without calculating all brackets in between?

@yume-chan yume-chan deleted the patch-1 branch September 30, 2021 08:42
@hediet
Copy link
Member

hediet commented Sep 30, 2021

I wonder if it's not possible to find the right bracket without calculating all brackets in between?

getAllDecorations is only called when disposing the model to clear some minimap cache. We really don't need to fetch all bracket pair colorization decorations here.

When rendering, only brackets that intersect with the viewport are fetched which are much less.

@github-actions github-actions bot locked and limited conversation to collaborators Nov 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

unable to open filename.js: Model is disposed!

2 participants