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

v3.2.0 freezes when editing a datatype #161

Closed
pepeiborra opened this issue May 13, 2020 · 16 comments
Closed

v3.2.0 freezes when editing a datatype #161

pepeiborra opened this issue May 13, 2020 · 16 comments

Comments

@pepeiborra
Copy link

In my case the datatype is something like:

data BenchResult costs = BenchResult costs

This was working fine with 3.0.3. I cannot seem to figure out how to rollback, since I'm also using ghcide and VSCode refuses to use an older version of language-haskell with ghcide.

The only special thing about my setup is that I'm using the VSCode remoting SSH plugin.

@sheaf
Copy link
Collaborator

sheaf commented May 13, 2020

Thanks for the bug report.
Is there any chance you could share the full datatype definition that causes the issue, and what editing action specifically causes a problem?

I have noticed an issue which has to do with the regex engine in VS Code having some problems with recursive calls, which I reported here. Note also that a recent update of the regex engine (included with VS Code version 1.45, april 2020) has mitigated the problem on my end.

@pepeiborra
Copy link
Author

pepeiborra commented May 13, 2020

The datatype definition was really what I shared above, in the context of a medium sized module. Typing the definition would cause a lockup.

I am still using 1.44 here, perhaps it’s better with 1.45

@sheaf
Copy link
Collaborator

sheaf commented May 13, 2020

I was able to reproduce with the following file:

data BenchResult 

instance ( Aaaaaaaaaa x ) where
instance ( Aaaaaaaaaa x ) where
instance ( Aaaaaaaaaa x ) where
instance ( Aaaaaaaaaa x ) where
instance ( Aaaaaaaaaa x ) where
instance ( Aaaaaaaaaa x ) where
instance ( Aaaaaaaaaa x ) where
instance ( Aaaaaaaaaa x ) where
instance ( Aaaaaaaaaa x ) where
instance ( Aaaaaaaaaa x ) where
instance ( Aaaaaaaaaa x ) where
instance ( Aaaaaaaaaa x ) where
instance ( Aaaaaaaaaa x ) where
instance ( Aaaaaaaaaa x ) where
instance ( Aaaaaaaaaa x ) where
instance ( Aaaaaaaaaa x ) where
instance ( Aaaaaaaaaa x ) where
instance ( Aaaaaaaaaa x ) where
instance ( Aaaaaaaaaa x ) where
instance ( Aaaaaaaaaa x ) where
instance ( Aaaaaaaaaa x ) where
instance ( Aaaaaaaaaa x ) where
instance ( Aaaaaaaaaa x ) where
instance ( Aaaaaaaaaa x ) where
instance ( Aaaaaaaaaa x ) where
instance ( Aaaaaaaaaa x ) where
instance ( Aaaaaaaaaa x ) where
instance ( Aaaaaaaaaa x ) where
instance ( Aaaaaaaaaa x ) where
instance ( Aaaaaaaaaa x ) where

Typing an equals sign just after data BenchResult should cause the syntax highlighting to update really slowly. For some reason this happens with Aaaaaaaaaa being at least 10 characters long; when it is 9 characters or less the issue is significantly less severe.

I'm investigating the issue at the moment; I will let you know when I have more info. Thanks for your patience.

@sheaf
Copy link
Collaborator

sheaf commented May 13, 2020

It seems the regex for highlighting a data constructor was backtracking excessively (on top of the already slow performance of recursive calls in VS Code that I alluded to above).

I've pushed a fix in 1bb6423, which on my end eliminates the slowness completely (at least for the above example).

@JustusAdam I think we should cut a new release, as this can be very frustrating.

@sheaf
Copy link
Collaborator

sheaf commented May 13, 2020

@pepeiborra In case you don't want to wait for a new release, you can replace your haskell.json file, located at $home/.vscode/extensions/justusadam.language-haskell-v3.2.0/syntaxes/haskell.json, with the following one which includes the aforementioned patch:
haskell.json.gz
(you'll need to restart VS Code for it to take effect).

Let me know if that fixes the problem on your end and we can go ahead with a new release. Thanks and sorry for the inconvenience.

@sheaf sheaf removed the info-needed label May 13, 2020
@pepeiborra
Copy link
Author

Thanks for the quick response! Will give it a go tomorrow and confirm here.

@pepeiborra
Copy link
Author

I updated back to v3.2.0 today but was unable to reproduce the problem, so I have no useful info to report. Nothing else has changed, yesterday it was 100% reproducible, today it just doesn't happen. I am perplexed.

@JustusAdam
Copy link
Owner

I am glad to hear that it has gotten better for you.

But just in case, I agree with you @sheaf, I'll publish a patch, just to be sure it doesn't affect more people.

@JustusAdam
Copy link
Owner

Hopefully the release fixed it for good

@sheaf
Copy link
Collaborator

sheaf commented May 15, 2020

Unfortunately I think my fix isn't right at all, here's another case that currently hangs the highlighting

data A 

xxxxxxxxxxxxxxxxx Aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb |
  ( Dddddddddddddddddddddddddd ( Eeeeeeeeeeeeeeeeeeeeeeeee fffffffffffffffffffffffff )
    ( Eeeeeeeeeeeeeeeeeeeeeeeee Fffffffffffffffffffffffff |
      ( Gggggggggggggggggggggg ( Aaaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbb, cccccccccccccccccccccccc )
        ( Dddddddddddddddddddddddddddd eeeeeeeeeeeeeeeeeeeeeeeeee |
          ( Hhhhhhhhhhhhhhhh iiiiiiiiiiiiiiiii )
        )
      )
    )
  )
xxxxxxxxxxxxxxxxx Aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb |
  ( Dddddddddddddddddddddddddd ( Eeeeeeeeeeeeeeeeeeeeeeeee fffffffffffffffffffffffff )
    ( Eeeeeeeeeeeeeeeeeeeeeeeee Fffffffffffffffffffffffff |
      ( Gggggggggggggggggggggg ( Aaaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbb, cccccccccccccccccccccccc )
        ( Dddddddddddddddddddddddddddd eeeeeeeeeeeeeeeeeeeeeeeeee |
          ( Hhhhhhhhhhhhhhhh iiiiiiiiiiiiiiiii )
        )
      )
    )
  )
xxxxxxxxxxxxxxxxx Aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb |
  ( Dddddddddddddddddddddddddd ( Eeeeeeeeeeeeeeeeeeeeeeeee fffffffffffffffffffffffff )
    ( Eeeeeeeeeeeeeeeeeeeeeeeee Fffffffffffffffffffffffff |
      ( Gggggggggggggggggggggg ( Aaaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbb, cccccccccccccccccccccccc )
        ( Dddddddddddddddddddddddddddd eeeeeeeeeeeeeeeeeeeeeeeeee |
          ( Hhhhhhhhhhhhhhhh iiiiiiiiiiiiiiiii )
        )
      )
    )
  )
xxxxxxxxxxxxxxxxx Aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb |
  ( Dddddddddddddddddddddddddd ( Eeeeeeeeeeeeeeeeeeeeeeeee fffffffffffffffffffffffff )
    ( Eeeeeeeeeeeeeeeeeeeeeeeee Fffffffffffffffffffffffff |
      ( Gggggggggggggggggggggg ( Aaaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbb, cccccccccccccccccccccccc )
        ( Dddddddddddddddddddddddddddd eeeeeeeeeeeeeeeeeeeeeeeeee |
          ( Hhhhhhhhhhhhhhhh iiiiiiiiiiiiiiiii )
        )
      )
    )
  )

Adding an equals sign after data A will cause a problem.

I'm having some difficulties specifying the intended behaviour in the regex. I want to say "stop trying this regex once you are sure you won't be able to match even after extending the ending", but I am finding it difficult to do that while preserving other desirable properties (particularly multiline highlighting and allowing intervening comments).

@sheaf sheaf reopened this May 15, 2020
@sheaf
Copy link
Collaborator

sheaf commented May 15, 2020

By the above comment I mean that we can add (?!\G) as an alternative at the end of the end regex, which will ensure we stop trying to match as soon as we have come across non-matchable code. This definitely works to prevent the catastrophic backtracking, but it also disables correct highlighting for:

data A =
  A B c

and

data A = {- ... -} A B c

sheaf added a commit that referenced this issue May 15, 2020
@sheaf
Copy link
Collaborator

sheaf commented May 15, 2020

I went for a different approach: write an explicit failure condition, that says "no point in trying further, just give up now". That has at least fixed the issues reported above. I'm closing the ticket for now, please reopen if other issues manifest themselves.

@sheaf sheaf closed this as completed May 15, 2020
@danieldjohnson
Copy link

I think I've run across another instance of this. Running syntax highlighting for the line

data Foo a = (:>) Bar a deriving (Show, Ord)

hangs forever at

@@scanNext 12: | (:>) Bar a deriving (Show, Ord)\n|

and a debugger indicates that it's hanging at findNextMatchSync for this rule. This happens both on the released version and on master.

@sheaf
Copy link
Collaborator

sheaf commented Jun 22, 2020

@hexahedria Thanks for the bug report, for the simple reproducer, and for checking on master. On my end it doesn't hang forever, but it slows down significantly and also produces incorrect highlighting.
I'm looking into it.

@sheaf sheaf reopened this Jun 22, 2020
sheaf added a commit to sheaf/language-haskell that referenced this issue Jun 22, 2020
  * Fixes some problems with slow/frozen highlighting (see JustusAdam#161)
@sheaf
Copy link
Collaborator

sheaf commented Jun 22, 2020

Turns out there was a silly typo in the regular expression for data constructors (see 05bfeff).
Fixed now, with some extra tests. @hexahedria let me know if you run into any further issues. Thanks again!

@sheaf sheaf closed this as completed Jun 22, 2020
@danieldjohnson
Copy link

Thanks for the quick fix!

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

4 participants