0

I have a method that gets called many times, repeatedly. 

public FilesCollection loadCollectionFromDirectory(string directory) 
        {
            string errorMessage;
            FilesCollection collection = new FilesCollection(_collectionType);

            string[] files = Directory.GetFiles(directory);
            foreach (string file in files)
            {
                if(file.EndsWith(".dll"))
                    collection.AddNewFileToCollection(file, out errorMessage);
            }
            files = null;
            return collection;
        }

I'm pretty sure the method calling this method is sucking up a ton of memory because of the string array that gets initialized every single time this method runs. From what I understand, each time this method is run again, the string array is set to a new collection of strings and the old strings remain in memory until garbage collection gets to them. Is that correct? Garbage collection isn't getting to them fast enough, as far as I can tell. There are numerous other problems affecting the Out Of Memory exception I'm getting but I think too many "ghost strings" is one of the big causes. If I set files = null like I'm doing right now, will that result in GC occurring earlier? Have I misunderstood the fundamentals of garbage collection and memory management?

I suppose my biggest question is - if I do string[] files = Directory.GetFiles(directory) over and over without doing something about the array, will this result in a bunch of ghost strings clogging up my system?

HandleThatError
  • 598
  • 7
  • 29
  • 4
    Possible duplicate of [C#: should object variables be assigned to null?](http://stackoverflow.com/questions/3903878/c-should-object-variables-be-assigned-to-null) – Mark Pattison Dec 08 '15 at 21:59
  • 1
    The array referenced by `files` becomes unreachable at the same time either way... the last time the loop reads from it. It's already eligible for collection before your `files = null;` line is hit. – Ben Voigt Dec 08 '15 at 22:00
  • @BenVoigt So it should be collected automatically? One of the issues with my system according to a memory profiler tool inside of Visual Studio is that there are many String types allocated over the course of the program's run. I'm thinking these are part of a problem I'm having with the system hanging after an Out Of Memory exception. – HandleThatError Dec 08 '15 at 22:02
  • @HandleThatError But your problem is not in the code you have posted. How can we help to fix it? – Eser Dec 08 '15 at 22:03
  • @Eser Understood, I'll keep searching through the codebase. I think I need to take a step back and learn how objects are allocated and deallocated, and how to properly initialize objects so that there aren't ghosts left behind. – HandleThatError Dec 08 '15 at 22:06
  • 1
    If an object goes out of scope the memory will be reclaimed whenever the Garbage Collector "feels like it". In case of your 'files' array it will go out of scope when leaving the method so no need to set it to NULL as there are no further references to it. What happens with that "FilesCollection" that you return? – Mark Dec 08 '15 at 22:09
  • Yes, like Mark asked, what happens with that "FilesCollection" that you return? Can you explain that? – Dieter Meemken Dec 08 '15 at 22:23
  • It gets passed around as a custom collection, and depending on what is going on in the application each file might be processed in its own thread or the collection might simply be kept on-hand to check if a certain file or directory exists in the collection. It isn't just one thing. It's hard to tell what is happening in its entirety, as it's a large codebase. – HandleThatError Dec 08 '15 at 22:27
  • Ah, that smells, don't you think. May be it is very long living, even in different threads. – Dieter Meemken Dec 08 '15 at 22:40
  • It's my first time working with a memory leak, and it's a pretty nasty one that four engineers before me couldn't figure out, so yes, I do think that I sense a foul scent wafting about – HandleThatError Dec 08 '15 at 22:43
  • So, can you provide more code or is it out of scope? Then good luck... – Dieter Meemken Dec 08 '15 at 22:51
  • Considering that my profiling tool shows that some of the heaviest allocation happens at the CLR .dll level...I would say that's out of scope. :( – HandleThatError Dec 09 '15 at 00:11

2 Answers2

2

Making files = null will make the string collection available for garbage collection but not free up the memory immediately. Garbage collector is called when the memory is running low. If within the scope of function file[] is too large, one way would be to get files only that you need. 

    public FilesCollection loadCollectionFromDirectory(string directory) 
    {
        string errorMessage;
        FilesCollection collection = new FilesCollection(_collectionType);

        string[] files = Directory.GetFiles(directory,"*.dll");
        foreach (string file in files)
        {
           collection.AddNewFileToCollection(file, out errorMessage);
        }
       // files = null;
        return collection;
    }

Whether you make it null or not, as the function scope ends the local variable files will be available for garbage collection.

diceguyd30
  • 2,742
  • 20
  • 18
goeldeepika3
  • 91
  • 4
  • Good suggestion regarding the second argument to GetFiles. I'll give this a try. – HandleThatError Dec 08 '15 at 22:18
  • @HandleThatError Also, if you're working with large sets of files, `Directory.EnumerateFiles` might be more efficient than `Directory.GetFiles`. – vesan Dec 08 '15 at 22:49
1

I am not that sure, but why don't you try to use List<string> instead of string[]. That should do the trick about the memory :) Please let me know if that worked for you.

test player
  • 333
  • 4
  • 14
  • 4
    This will not have an effect on memory consumption, since `List` is basically a wrapper around an array, and all the elements will remain in memory. – Martin Wiboe Dec 08 '15 at 22:16
  • 2
    Thank you, I always thought that the List are a bit better when you talk about memory :) – test player Dec 08 '15 at 23:01