Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve method ProcessInMemmory in AnalyzeCommand. #104

Closed
ArturLavrov opened this issue Jan 25, 2020 · 1 comment
Closed

Improve method ProcessInMemmory in AnalyzeCommand. #104

ArturLavrov opened this issue Jan 25, 2020 · 1 comment

Comments

@ArturLavrov
Copy link

Hello!
Thank's for this project. I found it useful! You guys did a great job!

Is your feature request related to a problem? Please describe.

Some people faced with an OutOfMemmoryException during the execution of AnalyzeCommand
#103, #91 .

I dived into this class(AnalyzeCommand.cs) and find a code, that I think could be improved.

I'm talking about method ProcessAsFile(string filename) in AnalyzeCommand.cs.

void ProcesAsFile(string filename)
        {
            if (File.Exists(filename))
            {
                _appProfile.MetaData.FileNames.Add(filename);
                _appProfile.MetaData.PackageTypes.Add(ErrMsg.GetString(ErrMsg.ID.ANALYZE_UNCOMPRESSED_FILETYPE));

                string fileText = File.ReadAllText(filename);
                ProcessInMemory(filename, fileText);
            }
            else
            {
                throw new OpException(ErrMsg.FormatString(ErrMsg.ID.CMD_INVALID_FILE_OR_DIR, filename));
            }
        }

I'm confused with these lines:

string fileText = File.ReadAllText(filename);
ProcessInMemory(filename, fileText);

Here we read a whole file content into memory using ReadAllText method and then call method ProcessInMemmory which verify if a length of the string exceded specified threshold:

if (fileText.Length > MAX_FILESIZE)
{
   //Some stuff goes here.
   return;
}

Fact, that we perform this operation in a loop, for each file trouble me a little bit.
Potentially this could create memory pressure.

Describe the solution you'd like

What if before reading the whole file into memory we will check the actual file size and only after that decide proceed file or not.
I think this could be achieved using FileInfo.Length property.

What do you think of this?
Feel free to correct me if I'm mistaken or didn't notice something.

Have a nice day!

guyacosta added a commit that referenced this issue Jan 25, 2020
Fix for #104 idea submitted by ArthusLavrov to optimize file size che…
@guyacosta
Copy link
Contributor

Fantastic catch. Note the reason the ProcessAsFile turns around and calls ProcessInMemory is to optimized the core processing logic that is used for both compressed and uncompressed files but by placing the file size check in the Run loop I can avoid opening those large files to begin with. I do have to check again for files once decompressed but that is far better than reading large files then finding out we didn't want to. Hopefully this will improve results for #103 and #91.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants