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

Apply timeout to simplification #942

Merged
merged 1 commit into from
Aug 28, 2022

Conversation

GrahamTheCoder
Copy link
Member

@GrahamTheCoder GrahamTheCoder commented Aug 25, 2022

Problem

#877
A deeply nested single expression seemed to cause this issue.

Solution

Currently just skips simplification when it takes longer than the configurable timeout. This at least allows the conversion to proceed. The first few things I tried give me the sense this is going to be hard to actually fix, even with a PR to roslyn itself, so I'll merge the workaround for now and get that released

  • Get a benchmark test set up for this and experiment with different approaches:
    • Simplify leaf nodes first and work up.
    • Skip simplifying subparts of a name, just the outer one may be enough
    • Try adding new lines
    • Try extracting sub expressions into their own field

Notes

@GrahamTheCoder GrahamTheCoder marked this pull request as ready for review August 28, 2022 22:35
@GrahamTheCoder GrahamTheCoder merged commit dddd15b into master Aug 28, 2022
@GrahamTheCoder GrahamTheCoder deleted the simplification-performance-fix branch January 31, 2023 00:49
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.

1 participant