rubberduck-vba / Rubberduck

Every programmer needs a rubberduck. COM add-in for the VBA & VB6 IDE (VBE).
https://rubberduckvba.com
GNU General Public License v3.0
1.92k stars 302 forks source link

Slows down VBA #2609

Closed Falthazar closed 5 years ago

Falthazar commented 7 years ago

I'm not sure if its my computer or something else. But whenever rubberduck is loaded it really slows down the VBA IDE. It seems to happen most when I press enter a few times, or when scrolling.

If I unload rubberduck it goes away.

retailcoder commented 7 years ago

How much RAM / CPU (horse-power?) do you have?

Because the VBIDE API's CodePane object exposes no events at all, in order to enable/disable menu commands depending on context (current selection), and to be able to update the "context status" in the RD commandbar, we had to register some kind of mouse+keyboard hook (on top of hotkey hooks) to determine when the selection in the code pane is changing.

We pick up left / right mouse button clicks in the code pane:

    private void Mouse_RawMouseInputReceived(object sender, RawMouseEventArgs e)
    {
        if (e.UlButtons.HasFlag(UsButtonFlags.RI_MOUSE_LEFT_BUTTON_UP) || e.UlButtons.HasFlag(UsButtonFlags.RI_MOUSE_RIGHT_BUTTON_UP))
        {
            OnMessageReceived(this, HookEventArgs.Empty);
        }
    }

And a specific set of keys:

    // keys that change the current selection.
    private static readonly HashSet<Keys> NavKeys = new HashSet<Keys>
    {
        Keys.Up, Keys.Down, Keys.Left, Keys.Right, Keys.PageDown, Keys.PageUp, Keys.Enter
    };

    private void Keyboard_RawKeyboardInputReceived(object sender, RawKeyEventArgs e)
    {
        // note: handling *all* keys causes annoying RTrim of current line, making editing code a PITA.
        if (e.Message == WM.KEYUP && NavKeys.Contains((Keys)e.VKey))
        {
            OnMessageReceived(this, HookEventArgs.Empty);
        }
    }

In the App class, we handle this MessageReceived hook event by re-evaluating the current code pane selection:

    private void _hooks_MessageReceived(object sender, HookEventArgs e)
    {
        RefreshSelection();
    }

This is probably the part responsible for the sluggish-ness:

    private void RefreshSelection()
    {
        var pane = _vbe.ActiveCodePane;
        {
            Declaration selectedDeclaration = null;
            if (!pane.IsWrappingNullReference)
            {
                selectedDeclaration = _parser.State.FindSelectedDeclaration(pane);
                var refCount = selectedDeclaration == null ? 0 : selectedDeclaration.References.Count();
                var caption = _stateBar.GetContextSelectionCaption(_vbe.ActiveCodePane, selectedDeclaration);
                _stateBar.SetContextSelectionCaption(caption, refCount);
            }

            var currentStatus = _parser.State.Status;
            if (ShouldEvaluateCanExecute(selectedDeclaration, currentStatus))
            {
                _appMenus.EvaluateCanExecute(_parser.State);
                _stateBar.EvaluateCanExecute(_parser.State);
            }

            _lastStatus = currentStatus;
            _lastSelectedDeclaration = selectedDeclaration;
        }
    }

Notice that on top of running FindSelectedDeclaration, when ShouldEvaluateCanExecute returns true, then every single menu command runs a number of checks to determine whether it should be enabled or disabled - this normally only takes a few milliseconds per command, but it does add up, and it does mean quite a lot of work is getting done every time the ENTER key is pressed in a code pane.

Merely scrolling shouldn't slow anything down though.

But without all this work, we couldn't tell you that you're selecting a constant, that it's declared in VBE7.DLL under the Strings module, and that it's referenced in 12 places in your code; we would have to leave all menus enabled all the time, and pop message boxes telling you that "the current selection is invalid for the selected command", for example when you click "refactor/encapsulate field" without selecting a field declaration.


We have a plan for v3.0, to literally hijack the code pane, and make it expose proper events and properties - that way we'll know without any key/mouse hooks, not only whether the selection changed, but also whether the code changed. This will open up a whole set of new possibilities, and make everything more efficient.

retailcoder commented 7 years ago

I'm leaving this issue opened, tagging with [performance] - let's try to improve performance of that critical code path.

Falthazar commented 7 years ago

System specs: 8 gigs of ram, Win 7 CPU is an i5 @ 1.9Ghz? Which is slow, I'm just looking at sys info.

It looks like it happens whenever i move the carat. As in if I click on a different line it'll lag for a bit. Something to do with the parser?

Edit: Also, not sure if related but Excel is using 422,000 K of ram right now. Not sure if due to my shitty code or something else.

retailcoder commented 7 years ago

Please read my first response about the lag.

It's not your code; Rubberduck collects and caches quite a lot of data, not only about your code, but also about every single referenced type library. On my machine a freshly started EXCEL.EXE process with an empty 3-sheet workbook eats up about 33,000K. If I load the VBE (and Rubberduck), it goes up to just over 80,000K. When I request the initial parse (without any code anywhere), it goes up close to 150,000K. If I add a new empty workbook (Rubberduck then parses again), it stabilizes at around 165,000K.

It's possible that a memory leak was (accidentally?) fixed between 2.0.11 and the current 2.0.12 build we're working with - in my IDE when I click the refresh/reparse button and monitor RAM usage in task manager, I don't notice memory usage going up after Ready state is reached (it does go up while processing though).

If memory consumption goes up while parsing, and doesn't go back to about what it was before you requested the reparse, and keeps going up with every reparse, then you've identified a memory leak; I'd suggest keeping an eye on it and, when it reaches a point where you deem it enough, save your work, close Excel (you may need to manually kill the EXCEL.EXE process), then start fresh with a new host process. I believe the current/next version isn't having this problem though.

Falthazar commented 7 years ago

Okay, so I restarted excel and it doesn't seem to use more memory with each re-parse. It only goes up drastically when I open or close a workbook. If I then manually tell it to re-parse the memory goes back down, however. Don't think this is the issue?

And sorry, I glossed over your initial comment admittedly. Re-read it and it seems to make sense. Hopefully that will be fixed in 3. It's not too big of a deal for now, but can get slightly annoying. Thanks for all your help today! Still new to coding.

mattretzer commented 7 years ago

Hello, I am trying out Rubberduck for Unit-testing, and I am experiencing a fair amount of hanging and re-parsing as I write my tests-- I am having to open and close dummy documents as part of my testing, so maybe that is exacerbating it. Is there a way to manually re-'Pause' Rubberduck's parsing while I work and then un-Pause again when I am ready to run tests & check code?

retailcoder commented 7 years ago

@mattretzer which version are you using? I'm pretty sure 2.0.11 doesn't parse when code is running.. could be wrong though - next release definitely won't.

comintern commented 7 years ago

@mattretzer - At this point RD's unit testing framework falls firmly on the side of Unit Testing as opposed to Integration Testing. What makes this slightly more difficult in VBA is that a good chunk of the code is implicitly tied to the object model of the host, and that somewhat blurs the distinction between the two types of tests.

In "normal" unit testing scenarios you would isolate things like opening and closing documents from your tests by providing mocks for the tests to operate on. This may become less of a problem if\when RD offers a mocking framework. That's an open discussion on #1550.

argeedblu commented 7 years ago

Experiencing the same performance hit described by OP. I installed latest release (my first install of RD) yesterday (10-13-17).

retailcoder commented 7 years ago

@argeedblu with the 2.1.0 release being imminent, we'll be starting serious work on the custom code panes in the next couple of weeks. This will change the face of the VBE and open up a ton of feature opportunities (code block folding, tearable tabs, custom theming, in-editor inspection results / colored squiggles, better syntax highlighting, etc.), but it will also allow us to remove the keyboard and mouse hooks we're currently using to work around the fact that the VBIDE API offers absolutely nothing as far as code pane events are concerned: without these hooks we have no way of knowing what's going on, and we would have to leave all commands enabled all the time and reactively (as opposed to proactively) validating the current selection, a bit like v1.4.3 was doing.

Inarion commented 7 years ago

@retailcoder This is some seriously exciting stuff! I had to ask permission from IT to change/hack my VBE7.dll just to make the VBE a bit more eye-friendly. (Overall I love the vision you have for RD - and the actual features seem to be getting there step by step.)

Regarding slow-downs, I experienced something similar in conjunction with a rather large and pretty messy Excel project I inherited. (It's so badly written that enabling Option Explicit would have me sitting there a whole workday to fix things. At one point I broke everything, just by making function/sub calls explicitly byRef / byVal...) I assumed the slowdown was due to >1000 inspections firing on every reparse. But the slow-downs disappeared after I stopped inspecting that particular project, so maybe it's not the exact same issue.

retailcoder commented 5 years ago

Closing, as the raw keyboard & mouse hooks involved in 2.0-2.1 are no longer present - Rubberduck is now "listening in" to the VBE's message pump instead.