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

Editor and file list stops responding #1086

Closed
ghost opened this issue Dec 24, 2016 · 12 comments
Closed

Editor and file list stops responding #1086

ghost opened this issue Dec 24, 2016 · 12 comments

Comments

@ghost
Copy link

ghost commented Dec 24, 2016

Problem

Editor and file list stops responding.
One of the task has 12% CPU utilization.

Environment data

.NET Command Line Tools (1.0.0-preview2-1-003177)

Product Information:
Version: 1.0.0-preview2-1-003177
Commit SHA-1 hash: a2df9c2576

Runtime Environment:
OS Name: Windows
OS Version: 10.0.14393
OS Platform: Windows
RID: win10-x64

VS Code version: 1.8.1

  • Shell: 1.4.6
  • Renderer: 53.0.2785.143
  • Node: 6.5.0

C# Extension version: 1.6.1

Steps to reproduce

Create new C# code file and paste code below.

namespace Test
{
    using System;
    using System.Collections.Generic;
    using System.Linq.Expressions;

    public interface IMappingExpression<TDestination, TSource>
    {
        IMappingExpression<object, object> MapMember<TMember>(Expression<Func<TDestination, TMember>> destinationMember, Expression<Func<TSource, TMember>> sourceMember);
    }

    public class MappingExpression : IMappingExpression<object, object>
    {
        private readonly ICollection<IMapping<object, object, object, object>> profiles;

        public IMappingExpression<object, object> MapMember<TMember>(Expression<Func<object, TMember>> destinationMember, Expression<Func<object, TMember>> sourceMember)
        {
            return this;
        }
    }

    public interface IMapping<TDestination, TDestinationMember, TSource, TSourceMember>
    {
         Type From { get; }
         Type To { get; }
         Expression<Func<TDestination, TDestinationMember>> Destination { get; }
         Expression<Func<TSource, TSourceMember>> Source { get; }
    }
}

Then try to remove '>' character from this line

private readonly ICollection<IMapping<object, object, object, object>> profiles;
@DustinCampbell
Copy link
Member

Hmmm... this happens without a C# project, so it's like a problem in the syntax grammar.

@DustinCampbell
Copy link
Member

It seems like there's some sort of exponential performance issue in the VS Code syntax highlighting that is exposed by our current C# syntax grammar. This is easily reproduced like so:

  1. Open VS Code with C# extension installed.
  2. Create a new file and set the type to C#
  3. Paste the following code:
class C
{
  Type1<Type2<object> field;
}

It should be pretty much instantaneous.

  1. Paste this code:
class C
{
  Type1<Type2<object, object> field;
}

It'll take longer, but the paste will succeed after a few seconds.

  1. Paste this code:

  2. Paste this code:

class C
{
  Type1<Type2<object, object, object> field;
}

Now it takes so long that the window stops responding.

@DustinCampbell
Copy link
Member

@jrieken, @alexandrudima, this clearly an issue with the syntax grammar. Do either of you have any ideas for how to diagnose this problem? Is it a VS Code issue or a problem with the grammar itself?

@DustinCampbell
Copy link
Member

DustinCampbell commented Dec 25, 2016

The troublesome regular expression appears to be: ([\w\.]+\s*<(?:[\w\s,\.`\[\]\*]+|\g<1>)+>(?:\s*\[\s*\])?). I threw this into http://www.rubular.com results in a warning with Type1<Type2<object> as a test string and got the following error:

Rubular suspects this regex will take forever to parse. Regexes of this sort make Rubular sad. Adjust the regex or else wait a few minutes and try again.

@DustinCampbell
Copy link
Member

cc @ivanz

@DustinCampbell
Copy link
Member

DustinCampbell commented Dec 25, 2016

Looking at the TypeScript grammar it appears that type names are handled quite a bit differently.

First, there's "type-name", which just has two patterns:

  1. ([_$[:alpha:]][_$[:alnum:]]*)\s*(\.) - This simply matches an identifier followed by a '.'.
  2. [_$[:alpha:]][_$[:alnum:]]* - This simply matches an identifier.

Then, there's "type-parameters", which has several patterns that reference #type (which references #type-name) and #punctuation-comma (which handles, well, commas).

This seems a better approach than using \g<1> to call subexpressions, which I strongly suspect is the performance issue in our grammar.

@DustinCampbell
Copy link
Member

DustinCampbell commented Dec 25, 2016

FWIW, I won't be able to look at this until the holiday is over. However, @pmatyja and @NinoFloris, to workaround the issue (for now), please do the following:

  1. Close all instances of Visual Studio Code

  2. Open the following file in an editor:

    • Windows: %UserProfile\.vscode\extensions\ms-vscode.csharp-1.6.1\syntaxes\csharp.json
    • OSX/LInux: ~/.vscode/extensions/ms-vscode.csharp-1.6.1/syntaxes/csharp.json
  3. Go to line 170, which should read:
    "match": "([\\w\\.]+\\s*<(?:[\\w\\s,\\.`\\[\\]\\*]+|\\g<1>)+>(?:\\s*\\[\\s*\\])?)",

  4. Delete the |\\g<1> from that line.

  5. Save and close the file.

That should address the performance issue though it'll leave some minor colorization issues in your code.

@DustinCampbell
Copy link
Member

I'm going to tweak the grammar in the release branch with that change and upload a new release.

@DustinCampbell
Copy link
Member

@ivanz: I've checked in the workaround for this issue. It's not great, and it required updating the test baselines, but the effect on the user-visible syntax highlighting isn't too bad. Essentially, there are now cases where outer <, > and , characters in a nested generic type don't get matched. So, I had to update several of the tests. I will take a look at fixing this more thoroughly and properly after the holiday.

@ivanz
Copy link
Contributor

ivanz commented Dec 25, 2016

Sorry about this gents. I will have a look at this after the holidays as well. The recursive regex should selectively be applied to very small sections of texts. Smells like it's not terminating when it should. Will have a look. Thanks for the quick workaround @DustinCampbell

@DustinCampbell
Copy link
Member

@ivanz: FYI that I've started work on a brand new C# syntax grammar here. Since we're going to make the C# syntax grammar official, I'd like to stop patching the old syntax and start fresh with something a bit closer to what TypeScript has.

@ivanz
Copy link
Contributor

ivanz commented Dec 31, 2016

I had a scan through your branch and it looks fantastic 👍 - great work @DustinCampbell ! Looking forward to seeing this merged :) Happy New Year everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants