Question Significant delay in code execution - any tips?

[ConsKa]

At the click of a button I check to see if an excel is open, if it is a message pops up to close it - once closed the following code is called:

1. Record all excel process IDs in a hashtable;
2. Export data to an excel spreadsheet;
3. Kill any excel processes that do not exist in the hashtable.

At the start of the Export Data to excel spreadsheet I have the following code:

OutPut.Text = "WORKING";
OutPut.BackColor = Color.DarkGreen;

That kind of tells me that the code has reached this point, as the button changes colour, almost instantly, and the text changes from Create Output to Working.

However, if you have the spreadsheet open, and you are told to close it. - after closing it...the time between the button changing and the dialog box opening is like 10 secs....if you do not have the excel open when you click output, the time is like a nano second.

The next line is simply:

if (File.Exists(ctk1.EX1))
{
    DialogResult dialogResult = MessageBox.Show
($"This will delete { ctk1.EX1 } are you sure?", "Delete Excel", MessageBoxButtons.YesNo);

It is the dialog box which just seems to take an age to display.

I tried a break point to see if it was going off to do something....but it isn't, it just hangs there for a long time.

Are there any tips on working out why there is such a delay, which might lead to a fix?

Below is the whole IF statement, just in case the way I have nested it is causing a problem.. It is the first DialogResult that is slow - once that fires, the rest first just fine with no delays.

if (OutPut.Text == "CREATE OUTPUT")
            {
                OutPut.Text = "WORKING";
                OutPut.BackColor = Color.DarkGreen;

                if (File.Exists(ctk1.EX1))
                {
                    DialogResult dialogResult = MessageBox.Show($"This will delete { ctk1.EX1 }, are you sure?", "Delete Excel", MessageBoxButtons.YesNo);
                    if (dialogResult == DialogResult.Yes)
                    {
                        File.Delete(ctk1.EX1);
                        MessageBox.Show("Daily Log Deleted");
                    }
                    else if (dialogResult == DialogResult.No)
                    {
                        OutPut.Text = "CREATE OUTPUT";
                        OutPut.BackColor = Color.Maroon;
                        return;
                    }
                }
                else
                {
                }

 

[ConsKa]

Well, splitting it into two methods (a check that it exists and deleting if agreed by user) and then calling the export method rather than doing two things in one method - which I know I shouldn't do....results in absolutely no delay regardless of whether the spreadsheet is open or not.

However, if anyone has any tips for finding out why there is a delay, would still like to hear them.

 

[Skydiver]

From my point of view, there are two possiblities:
1. Your machine is just underpowered to handle Excel, Visual Studio, and your app all running at the same time.
2. You have made your code re-entrant accidentally by using Application.DoEvents() somewhere in your code.

Why do you need a hash table of Excel process IDs? Shouldn't a list of process IDs suffice?

How are you exporting data to Excel? Are you using the Excel automation object model? Are you using a 3rd party library?

How exactly are you terminating the new Excel process? Are you letting it close gracefully, or are you just calling Process.Kill() instead of Process.CloseMainWindow()?

What is the bitness of your process? What is the bitness of the Office installation on the machine?

 

[Skydiver]

I just saw your post #2. Make me lean towards re-entrant code some more.

 

[ConsKa]

Hashtable for ID so that I can kill the created excel process without killing any other excel that are open....this was due to it always leaving a version of itself in the task manager.

Microsoft Interop for handling excel.

I don't think my Ryzen 9 3900x is the issue - but this needs to run on laptops so something to bear in mind. It really hasn't been process heavy though so far.

Just simply ExcelProcess.Kill();, it is a hanger on, it should close after the excel is fully closed, but it doesn't....so you have to kill it otherwise you can end up with like 20 of them in task manager and it can mess with you opening excels.

"I just saw your post #2. Make me lean towards re-entrant code some more."

I don't know, clearly, but simply moving the code to another method - I didn't change anything, except add a call to ExportDataToExcel(); which now doesn't have the testing/deleting IF statement I put above. I also don't have any do while loops in my code.

I considered one for the ExcelOpen(); test - but I simply did an IF and then called the method again ExcelOpen(); - I do want to fix that, but it works which is usually my first step...get it to work, then do it better.

 

[JohnH]

It looks like you are trying to solve the classic "memory release" problem with Office interop with dirty methods. Normally if you call Application.Quit in code the COM object will linger on until next GC, which could take a while, it will be released anyway after your application closes - unless your interop code crashed.

A simple clean way to do this I found in this thread: Application not quitting after calling quit - here's the method:

private void CleanupExcel()
{
    do
    {
        GC.Collect();
        GC.WaitForPendingFinalizers();
    }
    while (System.Runtime.InteropServices.Marshal.AreComObjectsAvailableForCleanup());
}

When I tested this it only took a few milliseconds and one loop for COM objects to clean up, about 1-2 seconds afterwards the Excel instance disappeared in Task Manager also.
What is important is that you call Application.Quit, and try-catch the interop code to ensure that you do. The CleanupExcel method also needs to be called after a method that does the interops returns. You could for example do it like this:

private async void button1_Click(object sender, EventArgs e)
{
    //could check/delete target file first
    await Task.Run(Excelworks);
    _ = Task.Run(CleanupExcel);
}

private void Excelworks()
{
    var app = new Excel.Application();
    app.Visible = false;
    try
    {     
        //do interop, save and close book
        var doc = app.Workbooks.Add();
        var sheet = (Excel.Worksheet)doc.ActiveSheet;
        sheet.Range["A1"].Value = "a value";
        var pathOut = System.IO.Path.Combine(Application.StartupPath, "output.xlsx");
        sheet.SaveAs(pathOut);
        doc.Close(false);
    }
    catch (Exception)
    {
        //nothing to do
    }
    app.Quit();
}

I just included some basic interop code in sample to show that no special memory release is used here, I'm referring to the endless calls to ReleaseComObject/set null/GC.Collect for any and all interop objects used that you probably have seen around the webs.

 

[Skydiver]

Hashtable for ID so that I can kill the created excel process without killing any other excel that are open....this was due to it always leaving a version of itself in the task manager.

I don't get it. So now not only are you hanging on to the ID of all the current running Excel processes, but you are also hanging on to all the Process object instances that represents those Excel processes. Those objects are not cheap. A process ID is just an integer and is cheap.

 

[ConsKa]

I don't get it. So now not only are you hanging on to the ID of all the current running Excel processes, but you are also hanging on to all the Process object instances that represents those Excel processes. Those objects are not cheap. A process ID is just an integer and is cheap.

I am creating a hashtable, I thought that would just be a table of hashes...not the process IDs and not process object instances?

Is that not right? I thought it was just a convenient way of storing and comparing a unique ID for each process instead of creating two lists to compare.

 

[ConsKa]

It looks like you are trying to solve the classic "memory release" problem with Office interop with dirty methods. Normally if you call Application.Quit in code the COM object will linger on until next GC, which could take a while, it will be released anyway after your application closes - unless your interop code crashed.

A simple clean way to do this I found in this thread: Application not quitting after calling quit - here's the method:

Thanks, I had seen this clean-up method referenced a few times, but it was always in very specific terms for a specific task - the last time I looked wasn't something I really 'got'.

I will look through your explanation though, which appears much clearer and see if I can change the code and get the same result.

 

[Skydiver]

I guess I misunderstood. In C#, a hashtable is a Dictionary<TKey,TValue>. With such a situation, presumably the key would be the process ID, and the value would be the process object.

But if you are using the word "hashtable" in the traditional way computer science classes teach how hashtables work they focus just on the key and never discuss the value, then yes that would be less expensive than holding on to the Process objects.

I highly recommend the method described above about letting Excel terminate gracefully. If you really much for a killing the Excel process, consider that you need to do is use LINQ's Except() to determine which ID is the new ID if you have a before and after list of IDs. And once you have that ID, do not use Process.Kill(). Use Process.CloseMainWindow() and only fallback on the Process.Kill() if the process still doesn't terminate after 2-3 minutes.

 

[ConsKa]

I guess I misunderstood. In C#, a hashtable is a Dictionary<TKey,TValue>. With such a situation, presumably the key would be the process ID, and the value would be the process object.

But if you are using the word "hashtable" in the traditional way computer science classes teach how hashtables work they focus just on the key and never discuss the value, then yes that would be less expensive than holding on to the Process objects.

I highly recommend the method described above about letting Excel terminate gracefully. If you really much for a killing the Excel process, consider that you need to do is use LINQ's Except() to determine which ID is the new ID if you have a before and after list of IDs. And once you have that ID, do not use Process.Kill(). Use Process.CloseMainWindow() and only fallback on the Process.Kill() if the process still doesn't terminate after 2-3 minutes.

Can I ask, what is the problem with just killing the process? Why is that bad?

What issues does it cause, or is just ugly or bad coding?

My issue with letting excel die off gracefully, is that I have had issues with that hanger on, if I try to open the file to check it is written.

If I kill the process, all of those issues disappear immediately, which is useful for the user because they aren't going to know about com objects requiring time to be cleared?

Admittedly, I have been using this code for awhile - and perhaps the hanger-on problem is related to when I was debugging more in the early stages and would hit issues and would crash with the hanger on....just not sure why getting rid of it immediately isn't a good thing.

 

[ConsKa]

I will look at the Linq Except() as it appears to do exactly the same job.

 

[ConsKa]

I wrote some code for the Linq - which I think does the same job as the lengthy code I had...

And they have gone, it does take a minute but they do go away.... my code if anyone wanted it after reading this discussion - this replaced 19 lines of code so a good shout....would still love to know why I shouldn't kill the damn thing

//Public variables
Process[] AllProcesses;
Process[] AllProcesses2;

// Called prior to running any interop code
AllProcesses = Process.GetProcessesByName("excel");

// Called after running the interop code
AllProcesses2 = Process.GetProcessesByName("excel");

IEnumerable<Process> createdExcel = AllProcesses2.Except(AllProcesses);

foreach (Process item in createdExcel)
    item.CloseMainWindow();

 

[Skydiver]

There's multiple reasons why you don't just go kill a process.

For starters, you are not giving the application a chance to shutdown and clean up anything it needs to clean up, or flush any files that it needs to write back out. Recall how in C++ it is a big no-no to exit() in the middle of a program? Do you recall the reason why? Well the reason is that system call just stops a program dead in its tracks. It doesn't give the program a chance to call any of its clean up code in its destructors. So basically the same thing happens when you just call Process.Kill(). You just stop a program dead in its tracks and don't let it do any of it's normal clean up and shutdown. Imagine if Excel were running and it happened to be in the middle of manipulating an Access database. Since the Access database engine runs in the same process space as Excel, by killing Excel, you are introducing a probability of corrupting the database -- not that Access needs any extra help corrupting its databases, it used to do that on its own quite well before the Office 2007 version . The same is true even if Excel were not playing with a database. With a the modern .xlsx file format, the internal XML data that goes into the .xlsx file needs to compressed and written out to a file. If Excel is still busy flushing out that file data when you kill it, you could end up with a corrupted file.

Another reason is that you are tearing down part of the COM/OLE remoting communications path. As you've discovered when you create an Excel automation object, Excel is actually running in a separate process, but you are making calls to Excel from code in your process. Kraig Brockschmidt has a much fuller explanation of how COM/OLE remoting works in his book Inside OLE, but basically what happens is that Excel starts up and exposes one ore more OLE object(s). That OLE object(s) are registered in the operating system's ROT (Running Object Table). In your code, a proxy that has the same interface as that Excel OLE object(s) are created. Then a communications channel is setup by COM/OLE so that the proxy object running in your process can talk to the actual object running in the Excel process. Since COM/OLE was created in the Win3.x days, TCP/IP was not common, Microsoft was avoiding named pipes because various Win3.x fiascos with named pipes, and shared memory was deemed too dangerous, the way the communications between the two processes is done by using Windows messages. (This is part of the reason why I have concerns of re-entrancy because if you mess up the message pump (most commonly by accidentally calling Application.DoEvents() in the wrong place, or uncommonly by having unusual window or dialog message handling), you also mess up all the COM/OLE communications. Been there. Done that. I have the CairOLE T-Shirt as well as owed the COM/OLE folks some dinner as they helped me debug why I kept on getting "COM Server Busy" issues.) So when you just kill the Excel process, Excel doesn't get a chance to remove itself from the ROT, and so COM/OLE keeps on trying to send messages and expect messages back from the now dead Excel COM object(s).

Another reason is telemetry and ruining one or more Microsoft engineer's day, week, or month. If you opted in to Office telemetry, then Office will report how many times Excel looks to have terminated abnormally. If you opted into Windows telemetry, Windows will also report which apps seem to have terminated abnormally and how many times. The data the Microsoft gets is anonymized and bucketed so the engineers looking at the incoming data won't be able to pinpoint that this issue seems to be with just one particular machine or user. All they get to see are counts, general information about the OS version, type of hardware, etc. If there's sufficient frequency for crashes/abnormal terminations, then someone has to investigate what could be possibly happening. Speaking from past experience, it was a mind numbing task. If the engineer is lucky, they will have a callstack, but in your case, you made like infinitely harder since you just terminated the app and so no callstack will be generated.

 

[ConsKa]

Thanks Skydiver, nice explanation - I followed your advice (as the post above demonstrates) simply because I know when to follow advice - but it is really nice to have a good explanation of why...rather than just blindly following.

Though now you just tempting me to implement a Kill.Process() on excel across every user I know just to mess with Microsoft