Performance issues 4.3.3.1 in Visual Studio 2010

Aug 8, 2010 at 2:21 AM

I was beginning to think the performance issues were limited to my 4 GB RAM/2GHz Dual Core/Vista/Gateway laptop, but after moving to a different machine

running Windows 7 with 6 cores and 8GB of RAM, it seems the problem is identical.

If I run StyleCop from the context menu (right clicking) inside the body of a C# file... after clicking on any of the "warnings" in the error window to go to the line of the "violation", performance begins to degrade. The CPU usage on the devenv.exe process goes above 50% on my dual core, and has a similar effect on the Windows 7/6 core machine.

Closing the output and error windows in VS 2010 seemed to have a small positive effect afterward, but very quickly, on both machines, it becomes impossible to work at all.

If I restart VS2010 on the Windows 7 machine, I can get performance back (I can edit all files needing style cop fixes before running style cop, and can work fully productively. Shutting down and restarting VS2010 on Vista doesn't seem to have as positive an effect on performance.

The key seems to be running stylecop from the right click menu inside a C# file in the text editor, and then double clicking on any of the violations in the error window (at the bottom of VS2010).

 

I did about 10 different virus scans and rootkit scans on my laptop to make sure it wasn't a virus affecting performance. After confirming through those routes that my computer is "virus free," I'm glad I don't have to re-install Vista on my laptop. But as the shop I'm working at requires StyleCop rules for all new code, and has it run in a hook for Subversion, we can't check in code unless it's StyleCop compliant.

We have hundreds of files that weren't developed with StyleCop, and making a small change to any of them can cause the entire file to be style copped... often there are hundreds of violations to fix in a single C# file.

 

If this is a known issue, please point me to any work arounds as it is severely affecting performance.

 

Thanks,

 

 

Chad Lehman

Basically Awesome Design LLC

Developer
Aug 10, 2010 at 5:29 PM

Have you tried to reproduce it in the latest version (4.4.0.14)?

Best regards,
Oleg Shuruev 

Developer
Aug 13, 2010 at 8:18 PM

Do you have any unmanaged projects in the solution?  This may be related to http://stylecop.codeplex.com/workitem/6662 ?  There is a lame workaround listed there involving being able to disable/enable StyleCop slightly more frequently than uninstalling it altogether.

Developer
Aug 24, 2010 at 9:30 PM

I believe I have fixed this in my styleCopKarakFork.  It would be interesting to see if this reproduces with that.  (This is my first contribution here so if we have an official process for getting attention/traction on it's integration, I'm not sure yet what that is.  I can only guess that if the code is acceptable that we'd see it in the mainline within a few days?  *shrug*)

Aug 25, 2010 at 9:43 PM

Hi Karak. I don't see any checkins from you in this fork. Perhaps you forgot to 'hg push'?



Developer
Aug 25, 2010 at 9:48 PM

Strange.  Sorry for my HG newbness.  I did a Commit, is that not the same thing?  (researching...)

Developer
Aug 25, 2010 at 9:59 PM

Found it, pushed it.  I recall now seeing that in the walk-through but obviously the information that 'push' was not also automatic with 'commit' didn't soak in at first, lol

Aug 26, 2010 at 4:13 PM

Hi Karak. I have reviewed your fix. I think in general the idea to use a 5 minute time is interesting, and the fix seem to be well implemented. However, I think that more may be needed here. The way that StyleCop determines whether a project should be included for analysis is by scanning through all the items in the project to see if it contains any items with a known file extension. Currently, since StyleCop only supports C#, it only looks for .cs files. You could actually add a .cs file to a C++ project, and StyleCop would find it and begin analyzing it.

Similarly, if you have a C# project which doesn't contain any C# files, StyleCop will ignore that project. When you add a C# file to the project, StyleCop begins including that project.

The timer solution doesn't take into account that a project may have changed in the previous 5 minutes since the cache was created. It would be weird if loaded a C# project which happened to not have any C# files in it, right-clicked on the project folder and saw that you didn't get the StyleCop menu items, then added a .cs file to the project, and you had to wait 5 minutes for the menu items to start working.

Probably the right solution is to do away with the timer, and instead register for the project changed event for each project. Then you would only need to clear the cache for a project each time the anything is added, removed, or renamed in the project. Does that make sense?

Thanks for tracking this down!

Jason

Developer
Aug 30, 2010 at 8:24 PM
Edited Aug 31, 2010 at 12:08 AM

> "more may be needed here.  ... the right solution is to do away with the timer, and instead register for the project changed event for each project"

Indeed, I mentioned as much in the issue page:  "Perhaps an ideal solution ... only refreshed when needed upon notable changes to that project. I don't know offhand if the COM interface supports eventing like that? (I just haven't worked with it before. I also don't have time to tackle this myself ATM so if someone wants to take a stab at it in the immediate future, now knowing what the problem is, just say so.)"  So with what you're saying it sounds like that'll be possible  :)

> It would be weird if loaded a C# project which happened to not have any C# files in it, ... had to wait 5 minutes for the menu items to start working

Although possible, it would be weird to load a C# project with no C# files in it.  This is a far, far less common scenario in production than "opening a large unmanaged solution".  Coupled with "mild 5minute annoyance" versus "completely crippling Visual Studio" I think we are certain that this checkin is helpful progress!  As is, 6662 completely cripples certain industries' ability to use the StyleCop product at all (and inflicting extreme headaches on the way to figuring out that it was even a StyleCop installation that was crippling their unmanaged workflow in the first place).  IE in the games industry it is common to have behemoth unmanaged-only solutions (a game) and have some separate C# solutions (tools) where StyleCop would be nice to have.

All I'm asking is that we take the fix for 6662 (so I can finally ask the rest of my coworkers to adopt StyleCop) and I will be happy to add a new issue to track... then attack the new, less significant bug (by replacing the cache with proper eventing), when I have more than a half hour block of time to address such an issue.  I'll try to set aside some time on an upcoming lunchtime to start researching how the COM eventing works. 

Actually, I didn't even have permissions to 'edit' and assign the existing issue to myself let alone mark it as fixed...  [edit: remove self-answered question]

Developer
Sep 23, 2010 at 9:59 PM

I've finally had some time to research the available events and implement this.  I have updated my branch with the recommended events invalidation approach.

Developer
Oct 1, 2010 at 9:23 PM
Edited Oct 1, 2010 at 9:24 PM

Bump...  wondering if this got missed or maybe our only individuals [hopefully we have more than one] with permission to integrate have been really busy / out on vacation / hit by a bus / etc?  Feedback?

Developer
Oct 25, 2010 at 9:20 PM

Bump?  Still waiting for a new code review / possible integration.  Is there another communication channel for this?  Or did I fail to "push" the committed changes again?

Oct 26, 2010 at 11:21 PM

Any updates on this? I am one of those in a "certain industry" (i.e. games) where I'd like to use StyleCop for WPF tool development, but it makes unmanaged deveopment in VS 2008 (required for Xbox development) unusable. If this isn't going to be taken soon, what is the easiest way to get a version of karak's fix?

Thanks,

 

Todd

Developer
Oct 27, 2010 at 8:56 PM

Worst case scenario for now: one can grab the source from my fork, build it, and replace installed StyleCop version with the bins:

http://stylecop.codeplex.com/SourceControl/network/Forks/Karak/styleCopKarakFork

Oct 28, 2010 at 11:34 PM

Hi Karak. Sorry for the delay on this. I have been out on vacation. I did review your pack and incorporated it into Main after a few small changes. Unfortunately, as we have not been able to reproduce this problem in-house, I was not able to test the fix. Would you please grab the latest source (as of changset e0a70bbcae47, and test this to make sure that the fix works properly?

If you and a few others can verify this, when I will create an official set of preview binaries containing this fix and host them up on the site.

Thanks!

Developer
Nov 1, 2010 at 9:39 PM

I'll pull it down and try to test it within a few days.

I am a little concerned about the removal of the locking in the followup change (http://stylecop.codeplex.com/SourceControl/changeset/changes/e0a70bbcae47)...

I have a couple assumptions, so maybe one of these is wrong:  Since projectEnabledCache is a Dictionary it is an IEnumerable.  Since it is IEnumerable, we have to take measures to ensure that the collection is not modified at the same time as it is being read.  Since the project change events are probably run from a different thread as other StyleCop code, we have to lock (at least certain) usages of projectEnabledCache to avoid race conditions.  .ContainsKey presumably either does, has, or may some day in the future, iterate the items (since we don't write that code, we have to assume the worst).  So having one thread .Clear during another's .ContainsKey may cause an exception when the .ContainsKey gets another time slice?  Sure it doesn't have to be it's own lock object since we can now lock the dictionary itself but I think we still need some locking there?

Developer
Nov 4, 2010 at 8:30 PM

I built and have been running with the linked version.  It was running fine for a while, having both a couple managed and one large unmanaged solution opened.  But now I'm getting the following error consistently: "SA0100: The file could not be read" even on files that I know I ran it against fine earlier today.

Developer
Nov 5, 2010 at 1:16 AM

LOL, ok so the bug repro is that it prints "SA0100: The file could not be read" whenever there are no violations now, hence why I initially had StyleCop working and then had that error (after I fixed the one violation).

On another note, it seems that having whitespace at the END of a file now raises "SA1518: The code must not contain blank lines at the start of the file" - if this is intended, either we should have the text changed to read something like "at the start OR end of the file", or IMO preferably add a new rule with a new error code, especially if there is a concern about changing that text (IE maybe that would invalidate some existing suppression attributes?)