8
\$\begingroup\$

I have the following static class that enumerates directories in a folder, then searches each file in the folder (it seems to only work with text files even thought I don't explicitly specify that) for a given string and returns an IEnumerable that holds the results.

It takes about 15 seconds to go through 40 text files that are about 250kb in size and I think it could be faster. Could I use a better algorithm, or is there a better method of achieving this?

public static class LogFileReader
{
    public static IEnumerable<string> GetLines(string path, string searchterm)
    {

        var dirs = Directory.EnumerateDirectories(path);
        List<string> thelines = new List<string>();

        foreach (var dir in dirs)
        {

            var files = Directory.EnumerateFiles(dir);


            foreach (var file in files)
            {
                using (StreamReader sr = new StreamReader(file))
                {
                    string line = string.Empty;
                    while ((line = sr.ReadLine()) != null)
                    {
                        if (line.IndexOf(searchterm, StringComparison.CurrentCultureIgnoreCase) >= 0)
                        {
                            thelines.Add(line);
                        }
                    }

                }
            }

        }

        return thelines;
    }
}
\$\endgroup\$
  • \$\begingroup\$ Are you reading from a local hard disc or from a network share ? \$\endgroup\$ – Heslacher Jul 28 '16 at 10:43
  • \$\begingroup\$ Hi, its a network drive i tested with. cheers \$\endgroup\$ – Nick Jul 28 '16 at 11:16
3
\$\begingroup\$

Currently your method does two things. It searches the directory structure and anylizes the files at the same time. This should be separated so that you can maintain each feature separately without affecting the other. For example should you want to search the directories recursively you now only need to change the GetFileNames method without thinking about reading the files.

public static IEnumerable<string> FindLines(this IEnumerable<string> fileNames, Func<string, bool> predicate)
{
    return fileNames.Select(fileName =>
    {
        using (var sr = new StreamReader(fileName))
        {
            var line = string.Empty;
            while ((line = sr.ReadLine()) != null)
            {
                if (predicate(line))
                {
                    return line;
                }
            }
        }
        return null;
    })
    .Where(line => !string.IsNullOrEmpty(line));
}

public static IEnumerable<string> GetFileNames(this string path)
{
    return 
        Directory.EnumerateDirectories(path)
        .SelectMany(Directory.EnumerateFiles);
}

If you want you can make it parallel later with:

var results = 
    @"c:\foo".GetFileNames()
    .FindLines(line => line.IndexOf("bar", StringComparison.CurrentCultureIgnoreCase) >= 0)
    .AsParallel()
    .ToList();
\$\endgroup\$
  • \$\begingroup\$ Hi thanks for your reply, but the code does not go into subdirectories. How can it be modified to do this please. I could be wrong but this looks like its using an anonymous method, so not sure how to modify. The previous methods posted return 1157 lines but this only returns 86. cheers \$\endgroup\$ – Nick Jul 28 '16 at 13:00
2
\$\begingroup\$

I think multithreading helps you to make you class faster. I rewrite it using Parallel class.

public static class LogFileReader
{
    private static object _lockObject = new object();
    public static IEnumerable<string> GetLines(string path, string searchterm)
    {
        var dirs = Directory.EnumerateDirectories(path);
        var result = new List<string>();

        Parallel.ForEach(dirs, dir =>
        {
            var files = Directory.EnumerateFiles(dir);
            Parallel.ForEach(files, file =>
            {
                var allFileLines = File.ReadAllLines(file);
                foreach (var line in allFileLines)
                {
                    if (line.IndexOf(searchterm, StringComparison.CurrentCultureIgnoreCase) >= 0)
                    {
                        lock (_lockObject)
                        {
                            result.Add(line);
                        }
                    }
                }
            });
        });

        return result;
    }
}

I created the log files(about 30) with roughly the same size and test performance before and after.
Before = 1250ms
After = 750ms

Also i have some minor suggestions about your code.

  1. It's a little strange that you look only in log files in subfolders and not in folder itself. Are you sure it is correct behavior ?
  2. It's strange that there is not any filter for log files(for extension, for example). I suppose it should be added
  3. I think variable name theLines is not correct from .Net Name convention point of view. i think lines will be better
  4. Too much empty lines in your code :)
\$\endgroup\$
  • 1
    \$\begingroup\$ those are not any lines, those are THE lines ;-) \$\endgroup\$ – t3chb0t Jul 28 '16 at 12:27
  • \$\begingroup\$ Thanks for your response and the tips. I tried this code and it was 2 seconds faster, but i noticed on the memory graph it used more memory than my code - 39mb compared to 25mb. is this a downside to consider when using parallel class or is it because the files are on a network drive? cheers \$\endgroup\$ – Nick Jul 28 '16 at 12:33
  • \$\begingroup\$ @Nick. Interesting question ... I don't know. Maybe you are right and it is because (at least partly) every thread has his own stack( with 1 or 2 Mb each) \$\endgroup\$ – Disappointed Jul 28 '16 at 12:51
1
\$\begingroup\$

As for the actual searching algorithm, it can probably improved on a few points:

  1. as you note, it doesn't work for binary files. Your output is the entire line the text appears on, which only makes sense for text files (binary files don't have lines, so that doesn't make sense). If you want to generalize it to work with binary files, you should just output the offset in the file that it was found at.
  2. depending on how long the lines are in your test files, you may also be getting a variable (bad!) amount of overhead from reading each line one at a time.
  3. to generalize the code and help level out the performance, you should read the file into a byte[] array, and then search through that. For RAM/Caching to not become a concern, have it read in blocks of a preset size (4KB? 64KB?) instead of lines, and then search each block.
  4. While indexOf is the fastest built-in way to search for a string in a pattern, there are of course many faster ways to search for a pattern, the simplest of which is the Boyer–Moore string search.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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