0
\$\begingroup\$

I am trying as a part of a bigger job to merge parts of several JPEGs into one huge BMP which will be processed further.

As a start, I tried to just copy a few JPEGs each to his own BMP file. In doing so I noticed that Visual Studio 2015 Community shows increased memory consumption if I repeat the copying of files in a single debug-run. The responsible part of the code is as follows.

Please, can you help me with this increasing memory consumption? In the future, I might need to read a few thousand JPEGs so this might be a serious issue.

private void GluePhotos(string[] files) {
    foreach (string file in files) {
        //Read original JPEG
        Stream imageSource = new FileStream(file, FileMode.Open, FileAccess.Read, FileShare.Read);
        JpegBitmapDecoder jpegDecoder = new JpegBitmapDecoder(imageSource, BitmapCreateOptions.PreservePixelFormat, BitmapCacheOption.Default);
        BitmapSource bitmapSource = jpegDecoder.Frames[0];

        //Prepare memory
        int stride = bitmapSource.PixelWidth * ((bitmapSource.Format.BitsPerPixel + 7) / 8);
        byte[] pixels = new byte[bitmapSource.PixelHeight * stride]; 

        //Copy JPEG into memory
        bitmapSource.CopyPixels(pixels, stride, 0);

        //Copy memory into a new BMP-to-be
        WriteableBitmap bitmap = new WriteableBitmap(bitmapSource.PixelWidth, bitmapSource.PixelHeight, bitmapSource.DpiX, bitmapSource.DpiY, bitmapSource.Format, bitmapSource.Palette);
        bitmap.WritePixels(new System.Windows.Int32Rect(0, 0, bitmapSource.PixelWidth, bitmapSource.PixelHeight), pixels, stride, 0);

        //Encode it as a BMP
        BmpBitmapEncoder bmpEncoder = new BmpBitmapEncoder();
        BitmapFrame newBitmapFrame = BitmapFrame.Create(bitmap);
        bmpEncoder.Frames.Add(newBitmapFrame);

        //Prepare paths for saving, save and cleaning(?)
        string dirPath = file.Insert(file.LastIndexOf('\\'), "\\ripoffs");
        dirPath = dirPath.Remove(dirPath.LastIndexOf('\\'));
        Directory.CreateDirectory(dirPath);
        string fileName = file.Insert(file.LastIndexOf('\\'), "\\ripoffs");
        fileName = fileName.Remove(fileName.LastIndexOf('.')) + "_ripoff.bmp";
        Stream imageOutput = new FileStream(fileName, FileMode.Create);
        bmpEncoder.Save(imageOutput);
        imageOutput.Close();
        imageSource.Close();
    }
}
\$\endgroup\$
8
  • 1
    \$\begingroup\$ Without even looking at your code my guess is you are not disposing \$\endgroup\$
    – paparazzo
    Nov 23 '16 at 15:54
  • \$\begingroup\$ @Paparazzi Close = Dispose, Dispose = Close \$\endgroup\$
    – t3chb0t
    Nov 23 '16 at 15:55
  • \$\begingroup\$ It is in C#, i.e. garbage collection is here, but I am not sure if I do not unintentionally keep something open/in memory/... and if yes, then what? \$\endgroup\$
    – Rao
    Nov 23 '16 at 16:00
  • \$\begingroup\$ Disposing of an object (closing) does not release memory. This is the way the C# garbage collector works! You aren't going to get an actual garbage collection until there is memory pressure. You can try to force this through a call to GC.Collect() but I would advise only for debugging/testing purposes. You may be trying to chase a problem that doesn't exist, try testing with multiple files, I think you'll find that it will increase to a point and then stabilize as the GC starts collecting. \$\endgroup\$
    – Ron Beyer
    Nov 23 '16 at 16:15
  • 2
    \$\begingroup\$ Use a memory profiler to answer questions about memory. \$\endgroup\$ Nov 23 '16 at 18:17
1
\$\begingroup\$

If you need comments to document the flow of your method, then you're doing it wrong. Obviously this needs to be split into much smaller methods, each dedicated to their own logic.

That was you could move the three lines below //Read original JPEG to a method of their own that would return a BitmapSource, and you could encapsulate imageSource in a using block (considering it's an IDisposable).

Same for imageOutput: another stream you're not properly disposing of.

Quite frankly, I'd move everything inside the foreach into a class of its own and start refactoring from there.


I had to look up what this does:

string dirPath = file.Insert(file.LastIndexOf('\\'), "\\ripoffs");

I can't help but feel that this is a hacky way of something that should be done with for instance FileInfo and Path etc;

Consider this (I'm assuming file is a global variable of the class that contains all of the code in the foreach):

    private string GetResultPath()
    {
        var fileInfo = new FileInfo(file);

        var directoryPath = fileInfo.DirectoryName;
        var newDirectoryPath = Path.Combine(directoryPath, "ripoffs");

        Directory.CreateDirectory(newDirectoryPath);

        var fileName = fileInfo.Name.Replace(fileInfo.Extension, string.Empty)
                                   + "_ripoff"
                                   + fileInfo.Extension;

        return Path.Combine(newDirectoryPath, fileName);
    }

Yes, that's longer, but it's also much clearer and informs the next developer who looks at that code what it does.

\$\endgroup\$
5
  • \$\begingroup\$ As for a first part, of your answer, I am a bit confused by the usingbehaviour. I thought that using(Stream s = ...) { ... } does the same as Stream s = ... //crunching ... s.close();. However, thanks for your ideas. I planned to go for refactoring later since this is my first attempt at manipulation of JPEGs/BMPs/... and converting them to and from. \$\endgroup\$
    – Rao
    Nov 23 '16 at 16:21
  • \$\begingroup\$ @Rao Yes, that does appear to be the case. However, you're leaving imageSource open for too long; no need to keep it open until the end. \$\endgroup\$
    – BCdotWEB
    Nov 23 '16 at 16:37
  • \$\begingroup\$ Try changing BitmapCacheOption.Default to BitmapCacheOption.OnLoad and see what that does for your memory footprint. \$\endgroup\$ Nov 23 '16 at 18:56
  • \$\begingroup\$ @Rao - no, using is equivalent to try { stream = ... } finally { stream.Dispose() }. I.e. Dispose will get called even if an exception is thrown. \$\endgroup\$
    – RobH
    Nov 24 '16 at 10:29
  • \$\begingroup\$ Oh, yes, Exceptions slipped from my mind. Thanks for the clarification! \$\endgroup\$
    – Rao
    Nov 25 '16 at 10:01
1
\$\begingroup\$

You are probably OK and it will be cleaned up in garbage collection
A stretch but maybe give this a try

public void bmap(string[] files)
{
    Stream imageSource;
    BitmapSource bitmapSource;
    int stride;
    byte[] pixels;
    WriteableBitmap bitmap;
    System.Windows.Int32Rect rect;
    BmpBitmapEncoder bmpEncoder;
    BitmapFrame newBitmapFrame;
    Stream imageOutput;
    foreach (string file in files)
    {
        imageSource = new FileStream(file, FileMode.Open, FileAccess.Read, FileShare.Read);            
        JpegBitmapDecoder jpegDecoder = new JpegBitmapDecoder(imageSource, BitmapCreateOptions.PreservePixelFormat, BitmapCacheOption.Default);
        imageSource.Close();
        bitmapSource = jpegDecoder.Frames[0];

        //Prepare memory
        stride = bitmapSource.PixelWidth * ((bitmapSource.Format.BitsPerPixel + 7) / 8);
        pixels = new byte[bitmapSource.PixelHeight * stride];

        //Copy JPEG into memory
        bitmapSource.CopyPixels(pixels, stride, 0);

        //Copy memory into a new BMP-to-be
        bitmap = null;
        bitmap = new WriteableBitmap(bitmapSource.PixelWidth, bitmapSource.PixelHeight, bitmapSource.DpiX, bitmapSource.DpiY, bitmapSource.Format, bitmapSource.Palette);
        rect = new System.Windows.Int32Rect(0, 0, bitmapSource.PixelWidth, bitmapSource.PixelHeight);
        bitmap.WritePixels(rect, pixels, stride, 0);

        //Encode it as a BMP
        bmpEncoder = new BmpBitmapEncoder();
        newBitmapFrame = BitmapFrame.Create(bitmap);
        bmpEncoder.Frames.Add(newBitmapFrame);

        //Prepare paths for saving, save and cleaning(?)
        string dirPath = file.Insert(file.LastIndexOf('\\'), "\\ripoffs");
        dirPath = dirPath.Remove(dirPath.LastIndexOf('\\'));
        Directory.CreateDirectory(dirPath);
        string fileName = file.Insert(file.LastIndexOf('\\'), "\\ripoffs");
        fileName = fileName.Remove(fileName.LastIndexOf('.')) + "_ripoff.bmp";
        imageOutput = new FileStream(fileName, FileMode.Create);
        bmpEncoder.Save(imageOutput);
        imageOutput.Close();
    }
}
\$\endgroup\$
1
  • \$\begingroup\$ Just one thing: I tried something similar but with usings. And if a ended before the line with CopyPixels, my output picture got corrupted. More precisely, part from beginning (line-wise) is okay, but after some point, it was only a colour (one for whole rest) and nothing more. Ending using after CopyPixels solved this. \$\endgroup\$
    – Rao
    Nov 25 '16 at 10:21

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.