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

Major performance regression in certain files starting with 1.9.4 -- 100% CPU for about 1 minute #218

Closed
sean-mcmanus opened this issue May 29, 2019 · 11 comments
Labels
High Priority Common or breaks highlighting of more than just itself

Comments

@sean-mcmanus
Copy link

sean-mcmanus commented May 29, 2019

Certain .cpp files make VS Code unusuably slow due to high 100% CPU usage on one core (extension host process) starting with 1.9.4 -- the issue doesn't repro with 1.9.3. It repros without our C/C++ extension. Could it be caused by https://github.com/jeff-hykin/cpp-textmate-grammar/pull/199/files ?

The repro we have has 2349 lines in it. Let me know if you guys need help getting a better repro.

I'm using the latest VS Code Insiders.

@sean-mcmanus
Copy link
Author

I ran the profiler -- it's showing vscode-textmate matchRule taking all the time.

@sean-mcmanus sean-mcmanus changed the title Major performance regression in certain files starting with 1.9.4 -- 100% CPU for over 1 minute Major performance regression in certain files starting with 1.9.4 -- 100% CPU for about 1 minute May 30, 2019
@matter123 matter123 added High Priority Common or breaks highlighting of more than just itself 🔍 investigating More information is being gathered labels May 30, 2019
@sean-mcmanus
Copy link
Author

sean-mcmanus commented May 30, 2019

I got the repro down to 91 lines (the CPU usage lasts only around 10 sec though). Invoking Developer: Inspect TM Scope causes major blockage "forever" until I reload.

I don't see anything unusual in the code.

@matter123
Copy link
Collaborator

The reproduction case would certainly be helpful. The only significant difference between v1.9.3 and v1.9.4 is support for default arguments in function definitions a048985, and all those patterns were preexisting.

@sean-mcmanus
Copy link
Author

sean-mcmanus commented May 30, 2019

Here's a repro -- I've replaced all the identifiers with "a". Freeze is 10-15 seconds -- more with Developer: Inspect TM Scope enabled. 1.9.3 has 0 seconds freezing. Repros with VS Code non-Insiders too.



static void a(
                           a    a,
                           a a,
                           a a,
                           a a,
                           a a,
                           bool                       a,
                           bool                       a,
                           bool                       a,
                           bool                       a)

{
  a a;


  a(a, a,
                                 a,
                                 a,
                                 a,
                                 a,
                                 a,
                                 a,
                                 a);
  a->a(&a);
  for (auto a : a) {
    auto &a = a.a;
    auto a = a.a;

    if (a && a) {
      switch (a) {
        case a:
          a->a(a->a());
          break;
        case a:
          a->a(a->a());
          break;
        case a:
          a->a(a->a());
          break;
        case a:
          a->a(a->a());
          break;
      }  
    }
    a(a, a,
                                   a,
                                   a,
                                   a,
                                   a,
                                   a,
                                   a,
                                   a);
  } 
}  

static void a(
                      a                    a,
                      a           a,
                      a a,
                      a                                 a)

{
  a a;
  a a =0;
                               a->a()->a();

  a(a->a());
  a->a(a);
  a->a(a);
  a(a, a, a,
                                 a,
                                 a);
  a->a(&a);
  for (auto a: a) {
    a(a->a(), a, a,
                                                                   a);
  }
}

@matter123
Copy link
Collaborator

matter123 commented May 30, 2019

@jeff-hykin The issue seems to be that function_definition is allowed inside of the function parameters. vscode-textmate spends about half of its time (4700ms) to determine that :function_definition is not a match. Do you know why its there?

Edit: :function_definition isn't slow because backtracking. wrapping the entire start_pattern in an atomic group causes a 1ms (4%) performance gain.

@matter123
Copy link
Collaborator

It looks like the performance issue is with generateScopeResolutionFinder. when the pattern is forced to be atomic the total time becomes .91s. This causes other issues with matching so it's not a solution, but hopefully, the scope resolution generator can be made non-backtracking.

@jeff-hykin
Copy link
Owner

vscode-textmate spends about half of its time (4700ms) to determine that :function_definition is not a match. Do you know why its there?

No I don't, and I can't think of a reason why it would intentionally be there. If removing it solves the issue, then we should definitely remove it.

It's still unclear to me why :function_definition would cause this, especially with backtracking disabled. It could be related to the look-arounds. I'm going to be without a computer tomorrow so I'm going to try to look into the issue now.

@jeff-hykin
Copy link
Owner

Thanks @sean-mcmanus for posting the example code

@jeff-hykin
Copy link
Owner

@matter123

This is strange
This code is not a problem:

static void a(
    bool a){
  a(a, a, a, a, a);
}

However this is near unusably slow:

static void a(
    bool                                                                     a){
  a(a, a, a, a, a);
}

Something is going on with the matching of whitespace.

@jeff-hykin
Copy link
Owner

jeff-hykin commented May 30, 2019

Fix pushed with version v1.11.2 @sean-mcmanus let us know if there are any cases that still have performance problems. Also just as an FYI, @matter123 just added a WIP performance metric to so we can catch performance issues in the future.

The fix corrects the :function_call_context and limits it's internal patterns to be much closer to what they should be. The performance issue seems to have disappeared.

:function_call_context was including :block and :block was including :$initial_context, so :function_parameter_context indirectly had more than every single pattern allowed in it, which is a problem in and of itself.

Now that the Objective-C and Objective-C++ syntaxes are fixed though, we can finally go through and clean up all the contexts and stop including :$initial_context inside of everything.

With that said, @matter123 I'm still not sure what was causing it to be so slow by simply adding whitespace. On another day I'm going to try to find which pattern was causing that.

@sean-mcmanus
Copy link
Author

Thanks, that fixed the issue with my 2.4k line file as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
High Priority Common or breaks highlighting of more than just itself
Projects
None yet
Development

No branches or pull requests

3 participants