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

Memory usage #39

Open
wvdvegt opened this issue Mar 10, 2022 · 8 comments
Open

Memory usage #39

wvdvegt opened this issue Mar 10, 2022 · 8 comments

Comments

@wvdvegt
Copy link

wvdvegt commented Mar 10, 2022

I added a

GC.Collect();

statement to the EveMonClient_TimerTick of mainwindow.cs (after the two udates methods) and it seems to lower the memory usage quite a bit (a steady 371M on my laptop). Without the forced garbase collection is peaks to over 0.5G before lowering to around 450M of memory in use.

@mgoeppner
Copy link
Owner

Generally calling GC.Collect() isn't a good habit in .NET -- have you hooked up the memory profiler in visual studio? That would be very helpful for debugging if there is a leak.

@wvdvegt
Copy link
Author

wvdvegt commented Mar 11, 2022

There does not seem to be a leak but just lots of memory allocated (Leaks in C# are very difficult without win32 api/interop/com usage). My observations come from the memory diagnostic tool.

My EveMon is very sluggish when moving it around the screen (the collect was one of the things i tried, altough the memory usage shrunk quite a bit, it did not solve my issue). Neither does the profiling the app. Basically I'm trying to find where it spend it's time

I'm aware of the collect not being something you should call but it's one of the ways to make garbagecollection a bit more aggressive.

@wvdvegt
Copy link
Author

wvdvegt commented Mar 11, 2022

Digging some more reveals that EveMonClient.TimerTick has quite some subscribers/handlers attached to it from all over EveMon.

EveMonClient.TimerTick .GetInvocationList().Count should get some info on what's going on. My guess is UpdateOnOneSecondTick() is trying to invoke them back2back as fast as possible,

[edit] TimerTick has 340-350 delegates attached to it which are executed every second.[/edit]

@mgoeppner
Copy link
Owner

EVEMon uses a lot of custom controls with overridden render methods -- I suspect if there is a performance issue when moving it, it could very well be related to that in addition to the subscribers/handlers. If you can get some performance logging when the stuttering happens, that would be very helpful for tracing down the root cause.

@wvdvegt
Copy link
Author

wvdvegt commented Mar 11, 2022

Times vary a lot (most are a few ms but some are much longer, I've seen up to 5000ms which should create issue with a 1s timer).

EventHandlerExtensions.ThreadSafeInvoke: 370 ms for 343 delegates
EventHandlerExtensions.ThreadSafeInvoke - ThreadSafeInvoke: 4 ms for 343 delegates

The profiler does not return useful data for me, not the single spot where the product times*time explodes.

I'll experiment a bit more with doublebuffering & disabling of redraws.

@wvdvegt
Copy link
Author

wvdvegt commented Mar 11, 2022

Left it to run for some hours and it becomes sluggish again (trails behind when moving the window around). All ThreadSafeInvoke are now at least 250+ ms. Also the code does not run every second anymore, which is strange.

0d 5h 36m 25s > EventHandlerExtensions.ThreadSafeInvoke - ThreadSafeInvoke: 359 ms for 350 delegates
0d 5h 36m 26s > EventHandlerExtensions.ThreadSafeInvoke - ThreadSafeInvoke: 260 ms for 350 delegates
0d 5h 36m 27s > EventHandlerExtensions.ThreadSafeInvoke - ThreadSafeInvoke: 202 ms for 350 delegates
0d 5h 36m 28s > EventHandlerExtensions.ThreadSafeInvoke - ThreadSafeInvoke: 224 ms for 350 delegates
0d 5h 36m 30s > EventHandlerExtensions.ThreadSafeInvoke - ThreadSafeInvoke: 279 ms for 350 delegates
0d 5h 36m 31s > EventHandlerExtensions.ThreadSafeInvoke - ThreadSafeInvoke: 297 ms for 350 delegates
0d 5h 36m 33s > EventHandlerExtensions.ThreadSafeInvoke - ThreadSafeInvoke: 942 ms for 350 delegates
0d 5h 36m 34s > EventHandlerExtensions.ThreadSafeInvoke - ThreadSafeInvoke: 266 ms for 350 delegates
0d 5h 36m 35s > EventHandlerExtensions.ThreadSafeInvoke - ThreadSafeInvoke: 265 ms for 350 delegates
0d 5h 36m 37s > EventHandlerExtensions.ThreadSafeInvoke - ThreadSafeInvoke: 280 ms for 350 delegates

@wvdvegt
Copy link
Author

wvdvegt commented Mar 12, 2022

Did some experimenting. Changed the code in EVEMon.Common\Extensions\EventHandlerExtensions.cs so it emits some timing data. I also added a Application.DoEvents() call to pump messages but not sure if really needed.

[edit] Without the DoEvents() execution times seem to be down to the 10ms range [/edit]

The top 3 where EVEMon\CharacterMonitoring\CharacterMonitor*.cs

In these classes methods UpdateFrequentControls() & UpdateInfrequentControls() I commented out the SuspendLayout()/ResumeLayout() pairs (and in \CharacterMonitorBody.cs also the Refresh() just after the ResumeLayout).

This creatly improved performance (around 70-80ms now).

Also note that in EVEMon.Common\Controls\ControlExtensions.cs I disabled the bodies of SuspendDrawing() and ResumeDrawing().

Finally in Dispatched.cs OneSecondTickTimer_Tick body I changed to:

s_oneSecondTimer.Stop();

            if (Monitor.TryEnter(locker))
            {
                try
                {
                    EveMonClient.UpdateOnOneSecondTick();
                }
                finally
                {
                    Monitor.Exit(locker);
                }
            }
            else
            {
                EveMonClient.Trace($"Skipped OneSecondTickTimer_Tick()");
            }

            s_oneSecondTimer.Start();

However there seems to be no overrung as the trace statement wasn't execute. This should indicate the timer happlily slips seconds every now and then.

public static void ThreadSafeInvoke(this EventHandler eventHandler, object sender, EventArgs e)
        {
            // Make sure we have some subscribers
            if (eventHandler == null)
                return;

            Stopwatch sw = new Stopwatch();
            sw.Start();

            Stopwatch time = new Stopwatch();
            List<KeyValuePair<long, string>> timing = new List<KeyValuePair<long, string>>();

            // Get each subscriber in turn
            foreach (EventHandler handler in eventHandler.GetInvocationList().Cast<EventHandler>())
            {
                time.Reset();
                time.Start();

                // Get the object containing the subscribing method
                // If the target doesn't implement ISyncronizeInvoke, this will be null
                ISynchronizeInvoke sync = handler.Target as ISynchronizeInvoke;

                // Check if our target requires an Invoke
                if (sync != null && sync.InvokeRequired)
                {
                    // Yes it does, so invoke the handler using the target's BeginInvoke method, but wait for it to finish
                    // This is preferable to using Invoke so that if an exception is thrown its presented
                    // in the context of the handler, not the current thread
                    IAsyncResult result = sync.BeginInvoke(handler, new[] { sender, e });
                    //veg
                    //Thread.Sleep(1);
                    sync.EndInvoke(result);
                    continue;
                }

                // No it doesn't, so invoke the handler directly
                handler.Invoke(sender, e);

                timing.Add(new KeyValuePair<long, string>(time.ElapsedTicks, $"{handler.Method.DeclaringType.FullName}.{handler.Method.Name}"));

                System.Windows.Forms.Application.DoEvents();

                //veg
                //Thread.Sleep(1);
            }

            sw.Stop();

            //! Print top 5 
            foreach (KeyValuePair<long, string> kvp in timing.OrderBy(p => p.Key).Reverse().Take(5))
            {
                EveMonClient.Trace($"{kvp.Key} - {kvp.Value}");
            }

            EveMonClient.Trace($"ThreadSafeInvoke: { sw.ElapsedMilliseconds} ms for {eventHandler.GetInvocationList().Length} delegates");
        }

@wvdvegt
Copy link
Author

wvdvegt commented Mar 14, 2022

Forget the line 'and in \CharacterMonitorBody.cs also the Refresh() just after the ResumeLayout'.

This Refresh() seems neccesary for the queue and skills lists to blink the actively trained skill.

Would be better to only paint the item being changed.

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

No branches or pull requests

2 participants