Memory Leak(?) with StreamReader

Question

I have a few very large files each of 500MB++ size, containing integer values (in fact it's a bit more complex), I'm reading those files in a loop and calculating the max value for all files. By some reason the memory is growing constantly during the processing, it looks like GC never releases the memory, acquired by the previous instances of lines.

I cannot stream the data and have to use GetFileLines for each file. Provided the actual amount of memory required to store lines for one file is 500MB, why do I get 5GB of RAM used after 10 files being processed? Eventually it crashes with Out of Memory exception after 15 files.

Calculation:

   int max = int.MinValue;

   for (int i = 0; i < 10; i++)
   {
      IEnumerable<string> lines = Db.GetFileLines(i);

      max = Math.Max(max, lines.Max(t=>int.Parse(t)));
   }

GetFileLines code:

   public static List<string> GetFileLines(int i)
   {
      string path = GetPath(i);

      //
      List<string> lines = new List<string>();
      string line;

      using (StreamReader reader = File.OpenText(path))
      {
         while ((line = reader.ReadLine()) != null)
         {
            lines.Add(line);
         }

         reader.Close();
         reader.Dispose(); // should I bother?
      }

      return lines;
   }

10 x 500 Mb = 5 GB. You are keeping all files contents in the list *List lines*. Same perf as ReadAllLines() — Cybermaxs, Oct 02 '12 at 11:24
Garbage Collector won't clear memory if you have pointers to the lines. You must clear() collection after lines were processed — opewix, Oct 02 '12 at 11:26
It is not so, cause there is always a new instance of list with each file and GC should clean up the last one — Murtuza Kabul, Oct 02 '12 at 11:27
@user1514042, I have provided you a solution that allows you to read the entire file at once, perform all processing, and then remove the memory used by storing the lines. — Mike Perrenoud, Oct 02 '12 at 11:45

cuongle · Accepted Answer · 2012-10-02T12:06:59.153

6

For very large file, method ReadLines would be the best fit because it is deferred execution, it does not load all lines in memory and simple to use:

  Math.Max(max, File.ReadLines(path).Max(line => int.Parse(line)));

More information:

http://msdn.microsoft.com/en-us/library/dd383503.aspx

Edit:

This is how ReadLines implement behind the scene:

    public static IEnumerable<string> ReadLines(string fileName)
    {
        string line;
        using (var reader = File.OpenText(fileName))
        {
            while ((line = reader.ReadLine()) != null)
                yield return line;
        }
    }

Also, it is recommended using parallel processing to improve performance when you have multiple files

edited Oct 02 '12 at 12:06

answered Oct 02 '12 at 11:23

cuongle

74,024
28
151
206

Sorry the processing logic is more complex the sample only demonstrates the fact that it's completely decoupled from reading process. – user1514042 Oct 02 '12 at 11:27
@user1514042: it does not matter much, you can use it with LINQ to process very large file – cuongle Oct 02 '12 at 11:28
Will it not kill the performance when the actual number of lines is 500K++? – user1514042 Oct 02 '12 at 11:29
Quite an elegant solution friend. – Mike Perrenoud Oct 02 '12 at 11:29
@user1514042, LINQ is just encapsulating the looping for you ... right? – Mike Perrenoud Oct 02 '12 at 11:29
@user1514042: yes, it deferred execution – cuongle Oct 02 '12 at 11:31
You still have to loop thru them all, don't ya to find the Max? – user1514042 Oct 02 '12 at 11:32
@user1514042, LINQ is doing that work for you. The functions that are being used a loops allowing you to operate over the collection. That's what LINQ does ... that's what it was built for. Am I missing something? – Mike Perrenoud Oct 02 '12 at 11:33
@user1514042: Yes, still loop thru to find Max but does not load all in memory to aoid leak – cuongle Oct 02 '12 at 11:33
The `File.ReadLines(path).Max(...` is like my `while` loop inside the `using` statement eh @CuongLe? I'm pretty sure I've got this right ... :) – Mike Perrenoud Oct 02 '12 at 11:36
@Cuong I put it on trial the results a are rather disappointing, please see the update. – user1514042 Oct 02 '12 at 11:55
@user1514042: I meant use `ReadLines` not `ReadAllLines` you are missing the point – cuongle Oct 02 '12 at 11:58

score 4 · Answer 2 · answered Oct 02 '12 at 11:21

4

You could be crashing because you are keeping references to the parsed result in memory after you are finished processing them (the code you show doesn't do this, but is that the same code you run?). It's highly unlikely that there's such a bug in StreamReader.

Are you sure you have to read all the file in memory at once? It might be quite possible to use an enumerable sequence of lines as IEnumerable<string> instead of loading up a List<string> up front. There is nothing that prohibits this, in this code at least.

Finally, the Close and Dispose calls are redundant; using takes care of that automatically.

answered Oct 02 '12 at 11:21

Jon

428,835
81
738
806

Well I only use value types, can they still hold the reference? – user1514042 Oct 02 '12 at 11:30
Of course they can. If you can somehow access the list, someone is holding a reference to it. – Jon Oct 02 '12 at 11:33
True, but it gets replaced everythime, your point would be right if I was unhappy final 500MB not being cleared, but I have a different problem. – user1514042 Oct 02 '12 at 11:43
1

@user1514042: If you are running out of memory, somewhere there are references that are not being cleared. It's that simple. – Jon Oct 02 '12 at 11:47
@user1514042, careful in your speech friend. You are certainly not managing memory the way you think you are or you wouldn't be running out of memory. Keep in mind that this line `IEnumerable lines = Db.GetFileLines(i);` literally copies the lists every time but **only** replaces the previous reference, therefore the previous `List` still exists on the heap. – Mike Perrenoud Oct 02 '12 at 11:49
@user1514042: Very much so. Perhaps you meant to address Mike instead? – Jon Oct 02 '12 at 12:12

score 1 · Answer 3 · answered Oct 02 '12 at 11:22

Why don't implement that as following:

int max = Int32.MinValue;
using(var reader = File.OpenText(path)) 
{
    while ((line = reader.ReadLine()) != null)
    {
         int current;
         if (Int32.TryParse(line, out current))
             max = Math.Max(max, current);
     }    
}

score 0 · Answer 4 · answered Oct 02 '12 at 11:21

0

You are reading the whole file into memmory (List lines )

I guess you could just read a line at a time and keep the highest number?

it will save you a lot of ram.

answered Oct 02 '12 at 11:21

Stig

1,323
16
22

Each line takes .5 sec to process, that's why it big time faster to read them up and then process. We gain a lot bu doing that, which is confirmed by performance tests. – user1514042 Oct 02 '12 at 11:26

score 0 · Answer 5 · answered Oct 02 '12 at 11:24

It appears that you are always loading entire file in the memory. At the same time, you are also creating managed objects (List) for each line of the file.

There is no reason that your memory usage will grow.

Please post rest of the code also, I doubt that you are somewhere having reference to this list which is in use and hence it is not being disposed.

Mike Perrenoud · Answer 6 · 2012-10-02T11:43:19.667

Alright, if you want a solution where you can read the entire file in at once, because you're sure that you need that performance gain, then let's do it like this so that way you don't have a memory issue.

public static int GetMaxForFile(int i) 
{ 
    string path = GetPath(i); 

    var lines = new List<string>(File.ReadAllLines(path));

    // you MUST perform all of your processing here ... you have to let go
    // of the List<string> variable ...
    int max = Math.Max(max, lines.Max(t=>int.Parse(t)));

    // this may be redundant, but it will cause GC to clean up immediately
    lines.Clear();
    lines = null;

    return max;
}

Memory Leak(?) with StreamReader

6 Answers6