Possible Memory leak? #123

Activity

1. dlemstra commented on Mar 5, 2017

   

   dlemstra

   on Mar 5, 2017

   Member

   More actions

   You are not disposing the image instances that you create. And I would advise you to reduce your code to a smaller sample that demonstrates we are not cleaning up memory properly. Your code example is way to big for us now.
2. Daniele122898 commented on Mar 5, 2017

   

   Daniele122898

   on Mar 5, 2017

   Last edited by Daniele122898

   Author

   More actions

   ´

               Configuration.Default.AddImageFormat(new PngFormat());
   
               using (var input = File.OpenRead($"{Context.User.Username}#{Context.User.Discriminator}BG.jpg"))
               {
                   using (var output = File.OpenWrite($"{Context.User.Username}#{Context.User.Discriminator}BGF.jpg"))
                   {
                       var image = new ImageSharp.Image(input);
                       //int divide = image.Width / 900;
                       //int width = image.Width / divide;
                       //int height = image.Height / divide;
                       image.Resize(new ResizeOptions
                       {
                           Size = new ImageSharp.Size(900, 10000),
                           Mode = ResizeMode.Max
                       });
                       //image.ExifProfile = null; TODO FIX THIS
                       //image.Quality = quality;
                       image.Save(output);
                       image.Dispose();
                       await input.FlushAsync();
                       input.Dispose();
                       await output.FlushAsync();
                       output.Dispose();
   
                       /*.Resize(new ResizeOptions
                           {
                               Size = new ImageSharp.Size(size, size),
                               Mode = ResizeMode.Max
                           });*/
                   }
               }
               //IMAGE RESIZE END
               if (File.Exists($"{Context.User.Username}#{Context.User.Discriminator}BG.jpg"))
               {
                   File.Delete($"{Context.User.Username}#{Context.User.Discriminator}BG.jpg");
               }
   
   
               if (userBG.ContainsKey(Context.User.Id))
               {
               }
               else
               {
                   userBG.TryAdd(Context.User.Id, true);
               }
   
               GC.Collect(); //TODO Change this system
   
               await Context.Channel.SendMessageAsync(":white_check_mark: Successfully set new BG!");
           }
           catch (Exception e)
           {
               Console.WriteLine(e);
               await SentryService.SendError(e, Context);
           }
       }`
   

   Oh sorry. Thats the important one. But im calling image.Dispose(); right after i saved it
3. antonfirsov commented on Mar 5, 2017

   

   antonfirsov

   on Mar 5, 2017

   Member

   More actions

   Using a using block would be much safer, than calling .Dispose() manually.

   I don't think we can reproduce your scenario from the sample above. If you are 100% sure ImageSharp-s code is leaking, it must be do so in a simple, short, standalone console application too. That would be very helpful, and definitely a thing we should investigate.
4. Daniele122898 commented on Mar 5, 2017

   

   Daniele122898

   on Mar 5, 2017

   Author

   More actions

   okey i will try to do that and post it in here then

   I just saw that imagesharp was the one in the Heap that was HUGE. That's why i thought that.

   But yes i'll try to do it in a smaller scale
5. tocsoft commented on Mar 5, 2017

   

   tocsoft

   on Mar 5, 2017

   Member

   More actions

   Have you tried performing it in a loop to see if the memory keeps growing or just grows once and doesn't release?
6. Daniele122898 commented on Mar 5, 2017

   

   Daniele122898

   on Mar 5, 2017

   Author

   More actions

   It's in a Discord bot so i just ran the command on different pics and different users and the ram usage only goes up once rly crazy. Like said from like 30mb to 70mb. Then when i run the command again it rises only by small amounts, usually around 4-8 mb.
   So it's mostly the first time the command get's run that the ram usage increases like crazy
7. tocsoft commented on Mar 5, 2017

   

   tocsoft

   on Mar 5, 2017

   Member

   More actions

   Sounds like its our use of an array pool for storing the image data... Basically the memory will be reused for the next image and should only grow the once.

   👍React with 👍1Reacted by Anton Firszov
8. Daniele122898 commented on Mar 5, 2017

   

   Daniele122898

   on Mar 5, 2017

   Last edited by Daniele122898

   Author

   More actions

   http://prntscr.com/egarm9

   Seems like it when i look at the heap snapshot
9. antonfirsov commented on Mar 5, 2017

   

   antonfirsov

   on Mar 5, 2017

   Last edited by antonfirsov

   Member

   More actions

   I think @tocsoft is right, it works as expected. The arrays managed by ArrayPool are being reused if you create images with same, or smaller sizes. We do this to avoid allocation overhead and LOH fragmentation

   If the "leaked memory" stops growing after the application is warmed up, I think you have nothing to be worried about.
10. Daniele122898 commented on Mar 6, 2017

    

    Daniele122898

    on Mar 6, 2017

    Author

    More actions

    that makes sense. So not having to reallocate the memory makes processing faster i assumse.

    Shall i close this issue then?
11. dlemstra commented on Mar 6, 2017

    

    dlemstra

    on Mar 6, 2017

    Member

    More actions

    I am now wondering if we should add a configuration setting where the user can enable or disable array pooling. Some systems might need the memory returned after usage.
12. JimBobSquarePants commented on Mar 6, 2017

    

    JimBobSquarePants

    on Mar 6, 2017

    Member

    More actions

    There's talk of the whole memory management thing pluggable.
13. JimBobSquarePants commented on Mar 6, 2017

    

    JimBobSquarePants

    on Mar 6, 2017

    Member

    More actions

    But yeah, close this. The behaviour is expected. ArrayPool<T> pulls a minimum of then reuses which is beneficial in most circumstances.
14. dlemstra

    closed this as completedon Mar 6, 2017