Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I wrote a C# program to remove non-ASCII characters in a text file, and then output the result to a .NonAsciiChars file.

The input file is in XML format. In fact, the data may all be on two lines, which is why I am not doing the replacement line by line. Instead, I'm using StreamReader.ReadToEnd().

The problem is, the input file can be as big as 4 GB. When this happens, I'm getting the following OutOfMemoryException.

DateTime:2014-08-04 12:55:26,035 Thread ID:[1] Log Level:ERROR Logger Property:OS_fileParser.Program property:[(null)] - Message:System.OutOfMemoryException: Exception of type 'System.OutOfMemoryException' was thrown.
   at System.Text.StringBuilder.ExpandByABlock(Int32 minBlockCharCount)
   at System.Text.StringBuilder.Append(Char* value, Int32 valueCount)
   at System.Text.StringBuilder.Append(Char[] value, Int32 startIndex, Int32 charCount)
   at System.IO.StreamReader.ReadToEnd()
   at OS_fileParser.MyProgram.FormatXmlFile(String inFile) in D:\Test\myProgram.cs:line 530
   at OS_fileParser.MyProgram.Run() in D:\Test\myProgram.cs:line 336

Line 530 contains content = Regex.Replace(content, pattern, "");, while line 336 calls a method with the following body:

const string pattern = @"[^\x20-\x7E]";

string content;
using (var reader = new StreamReader(inFile))
{
    content = reader.ReadToEnd();
    reader.Close();
}

content = Regex.Replace(content, pattern, "");

using (var writer = new StreamWriter(inFile + ".NonAsciiChars"))
{
    writer.Write(content);
    writer.Close();
}

using (var myXmlReader = XmlReader.Create(inFile + ".NonAsciiChars", myXmlReaderSettings))
{
    try
    {
        while (myXmlReader.Read())
        {
        }
    }
    catch (XmlException ex)
    {
        Logger.Error("Validation error: " + ex);
    }
}

How can I improve the memory footprint of my code?

share|improve this question
    
Original question, cross-posted here: stackoverflow.com/questions/25121754/… –  ANeves Aug 5 at 15:13
    
so this works for smaller files, but is not efficient on larger files? –  Malachi Aug 5 at 15:13
1  
@Malachi, yes, you are right, tested 10 files (less then 50MB) all working fine, but when I try to process the 2GB file, it went into OutOfMemoryException. –  user3724711 Aug 5 at 15:17
2  
Voting to reopen: this code is working, but is inefficient. Optimisation questions are on-topic for Code Review. (Incidentally, I retracted my Close Vote but it claimed I had no outstanding votes on the question... bug?) –  Schism Aug 5 at 15:18
1  
Have you tried simply running this in a 64-bit mode? –  svick Aug 5 at 17:26

3 Answers 3

up vote 7 down vote accepted

@svick's approach is the right one, given these considerations

  • the input file can be as big as 4 GB
  • the data may all be on two lines

However I would suggest that regular expressions are the wrong tool for the job, and you will find it faster to use a StreamReader with a specified encoding.

There is a method Encoding.GetEncoding that does the following:

Returns the encoding associated with the specified code page name. Parameters specify an error handler for characters that cannot be encoded and byte sequences that cannot be decoded.

There is also a DecoderReplacementFallback class:

Provides a failure-handling mechanism, called a fallback, for an encoded input byte sequence that cannot be converted to an output character. The fallback emits a user-specified replacement string instead of a decoded input byte sequence.

Putting that all together would look like this:

var encoding = Encoding.GetEncoding("us-ascii", new EncoderExceptionFallback(), new DecoderReplacementFallback(string.Empty));
const int BufferSize = 4096;
using (var reader = new StreamReader("in.txt", encoding))
using (var writer = new StreamWriter("out.txt", false, new UTF8Encoding(false), BufferSize))
{
    var buffer = new char[BufferSize];
    int read;
    while ((read = reader.Read(buffer, 0, BufferSize)) != 0)
    {
        writer.Write(buffer, 0, read);
    }
}

Now that use of UTF8Encoding might look like a mistake, but according to MSDN:

If you are planning to use an ASCII encoding (ASCIIEncoding), choose UTF8Encoding instead. The two encodings are identical for the ASCII character set, but UTF8Encoding has the following advantages:

  • It has been tuned to be as fast as possible and should be faster than any other encoding. Even for content that is entirely ASCII, operations performed with UTF8Encoding are faster than operations performed with ASCIIEncoding.

We use the constructor so that it doesn't emit a byte order mark, unlike Encoding.UTF8.


I wrote the code above based on your description "remove non-ASCII characters in a text file", but now realise your requirements are more specific: to remove characters outside the range 32-126. It can be easily modified to support that requirement:

var encoding = Encoding.GetEncoding("us-ascii", new EncoderExceptionFallback(), new DecoderReplacementFallback(string.Empty));
const int BufferSize = 4096;
using (var reader = new StreamReader("in.txt", encoding))
using (var writer = new StreamWriter("out.txt", false, new UTF8Encoding(false), BufferSize))
{
    var buffer = new char[BufferSize];
    var chars = new char[BufferSize];
    int read;
    while ((read = reader.Read(buffer, 0, BufferSize)) != 0)
    {
        var j = 0;
        for (var i = 0; i < read; i++)
        {
            char c = buffer[i];
            if (c >= 32 && c <= 126)
            {
                chars[j++] = c;
            }
        }

        writer.Write(chars, 0, j);
    }
}

Timing

I ran tests on a 4GB file with random content, generated by Dummy File Creator.

$ time tr -cd '\40-\176' < in.txt > out.txt

real    0m52.034s
user    0m24.897s
sys     0m22.791s

$ time ./AsciiCopy.exe in.txt out.txt

real    1m9.140s
user    0m0.015s
sys     0m0.031s

$ time ./AsciiCopyRegex.exe in.txt out.txt

real    17m59.618s
user    0m0.031s
sys     0m0.016s
share|improve this answer
    
thanks mjolka, on my VM it tooks 6 Mins to process 3.39 GB file. Amazing!!!, when I open by EmEditor Pro 32 Bit 12.0.11 I found TAB still inside the output file which is inside ASCII Table but I dont need it. is there any way to remove also? I want ASCII Dec32 to Dec 126 ONLY. thanks a lot. –  user3724711 Aug 7 at 16:52
    
@user3724711 I've updated the post. –  mjolka Aug 7 at 23:43
    
thanks a lot, the speed also amazing, on my VM it tooks 7 mins only, I believe in server it will be fantastic. thanks a lot. this is a really good solution. I will change to 'if ((c >= 32 && c <= 126) || c == 10 || c == 13)' to support Line Feed and Carriage Return also, which I forgot to mentioned. thanks. –  user3724711 Aug 9 at 15:38

You need to use the two Streams as, well, streams: read a manageable part of the input, transform it, write it to the output and repeat.

int bufferSize = 4096; // or whatever

char[] characters = new char[bufferSize];

using (var reader = new StreamReader(inFile))
using (var writer = new StreamWriter(inFile + ".NonAsciiChars"))
{
    while (true)
    {
        int read = reader.Read(characters, 0, characters.Length);

        if (read == 0)
            break;

        var replaced = Regex.Replace(new string(characters), pattern, string.Empty);

        writer.Write(replaced);
    }
}

Some notes about this code:

  1. Notice the missing Close() calls: the whole point of using is safe closing of streams, and similar resources, so you don't need to close them twice.
  2. This code (just like the original) creates a lot of garbage to be collected by the GC. Since your regex is actually very simple, it might be better to manually work directly with char[]s.
  3. I used string.Empty instead of "". This makes it very clear that empty string was actually intended and it's not just “I started writing string and then forgot about it” bug.
share|improve this answer
    
thanks svick, Seems it works, on my VM it tooks 18 Mins to process 3.39 GB file. only thing is when I use EmEditor Pro 32 Bit 12.0.11 to open the Output .NonAsciiChars file I am getting "Too many characters in a single line. Out of memory." not sure why... but file size did changed, seems it's good. –  user3724711 Aug 7 at 16:46

okay first of all, you need to use buffered stream to read data from the file, there is no need to write data once in file as it might hung altogether.

so I would suggest that read the file line by line , parse it and then write to file.

using (FileStream fileStream = File.Open(infile, FileMode.Open))
{
    using (var bufferedStream = new BufferedStream(fileStream))
    {
        using (var writer = File.AppendText(infile + ".NonAsciiChars"))
        {
            using (var stremReader = new StreamReader(bufferedStream))
            {
                string inputContent;
                while ((inputContent = stremReader.ReadLine()) != null)
                {
                    string content = Regex.Replace(inputContent, pattern, "");
                    writer.WriteLine(content); //you can replace with write too if you want
                }
            }
            writer.Close();
        }
    }
}
share|improve this answer
1  
thanks a lot. some time the file coming in ONE line only, when i use stremReader.ReadLine() same error popup at Regex.Replace(inputContent, pattern, ""); –  user3724711 Aug 5 at 16:12
    
that is why I am reading from buffered stream which is actually a chunk of data which fixed size , so you should not get any error –  paritosh Aug 5 at 16:19
3  
@paritosh That's not what BufferedStream does. When you ask for a line, you get a line, not something smaller. –  svick Aug 5 at 17:29
    
You are right and I apologise –  paritosh Aug 5 at 17:44
    
thanks alot, it works, when i run 3 GB file every min can process 0.05 GB the program still running but till now already 1.17 GB finished, not more outofmemory cames, thank you so much +1 –  user3724711 Aug 5 at 17:46

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.