Memory Leak

[mtgrares]

I still haven't figured out how to fix the memory leak problem completely though.

Any guesses on where the new memory leak is at?

I know Forge had a previous memory leak because Phase.nextPhase() was used to advance everything and the stack which keeps track of all the method calls would overflow because nextPhase() was continually used.

The problem is that the stack trace never stops, it just keeps getting larger. If AllZone.Phase was created at the beginning of every game

Code: Select all

AllZone.Phase = new Phase();

it would fix this problem and would certainly cause other problems. Even though the stack gets bigger and bigger, it only lasts for one match and it isn't a problem. The problem is after playing many matches that the stack keeps getting bigger and bigger.

(I guess I pushed the observer pattern to the extreme and sort of broke it. Using global static variables can be tricky because you can easily shoot yourself in the foot.)

You can use Thread.dumpStack() to view the current stack.

Stack errors are very hard to understand. I passed the Java Certified Programmer test but it didn't cover the stack at all.

(In Magic and Java the stack is hard to understand. )

[DennisBergkamp]

I think the main memory leak is in the picture displaying part... everytime a new picture gets displayed, a new JPanel is created... often this happens faster than the garbage collection system can take care of.

[nantuko84]

haven't you fixed that yet? I thought I made rather detailed description how to manage too many creation issue.

[DennisBergkamp]

Nope I haven't... I tried a bunch of things, but I can't figure out how to make it so the picture is drawn directly to the component
There is no such thing as JPanel.setComponent(component)...

[nantuko84]

first tried to download HQ pictures to try to reproduce the issue, but failed

I've created a bug in the bug thread. please take a look

[DennisBergkamp]

For some reason, in the pics_link/ folder, Zendikar cards are referred to as :

Code: Select all

cardName.jpg       http://pics.slightlymagic.net/ZEN/cardName.full.jpg

as opposed to:

Code: Select all

cardName.jpg       http://[server]/ZEN/cardName.full.jpg

There might be a reason for this, Vasiliy added these. But it explains why those downloads fail, if pics.slightlymagic is down.

[nantuko84]

here are the results of testing:

1. without any images:
I played 5 games starting with 76 Mb in task manager
once 5th was started, I had 78 Mb allocated that means no memory leak in the program

2. I've downloaded some images
started a new game (again with about 76 Mb allocated)
all cards were text except one (Nevinnyral's Disk, but it doesn't matter)

I put mouse over the card with image so it was displayed at the right, then enlarge it putting mouse cursor over image.
I repeated this 20 times and now I had 122 Mb allocated.

when I started a next game, and it had already 121 Mb allocated instead of starting 76 Mb - so here is the source of your memory leak

regards

[silly freak]

i'm unfortunately busy with school, so I can't code too much myself now, but these might help:

• JPanel can't "add images or such, but Graphics can draw them if you use a custom subclass of JPanel and override paintComponent()
• alternately, you can use a Label, which can display an Icon. ImageIcon is more or less a wrapper for image to icon.
• images can be stored using a Map<String, WeakReference<Image>>. keys could be <Cardname>/<Resolution> or something similar. WeakReference makes it available for the garbage collector if there's no "strong" reference to it (likely through the label).
• Don't use WeakHashMap, it's not what you want

[Snacko]

Weak references might be too weak if this is supposed to be a cache as they're going to be destroyed as soon as they're not used (no strong references). Soft references are more preferable, as they stick as long as there's memory available.

Here's also some nice Arcane GPLv3 code you can use to render images. It supports multipass / blurred images if you scale more than half down.

Code: Select all

package arcane.ui;

import java.awt.Graphics;
import java.awt.Graphics2D;
import java.awt.Image;
import java.awt.RenderingHints;
import java.awt.image.BufferedImage;

import javax.swing.JPanel;

public class ScaledImagePanel extends JPanel {
   public volatile Image srcImage;
   public volatile Image srcImageBlurred;

   private ScalingType scalingType = ScalingType.bilinear;
   private boolean scaleLarger;
   private MultipassType multiPassType = MultipassType.bilinear;
   private boolean blur;

   public ScaledImagePanel () {
      super(false);
      setOpaque(false);
   }

   public void setImage (Image srcImage, Image srcImageBlurred) {
      this.srcImage = srcImage;
      this.srcImageBlurred = srcImageBlurred;
   }

   public void clearImage () {
      srcImage = null;
      srcImageBlurred = null;
      repaint();
   }

   public void setScalingMultiPassType (MultipassType multiPassType) {
      this.multiPassType = multiPassType;
   }

   public void setScalingType (ScalingType scalingType) {
      this.scalingType = scalingType;
   }

   public void setScalingBlur (boolean blur) {
      this.blur = blur;
   }

   public void setScaleLarger (boolean scaleLarger) {
      this.scaleLarger = scaleLarger;
   }

   public boolean hasImage () {
      return srcImage != null;
   }

   private ScalingInfo getScalingInfo () {
      int panelWidth = getWidth();
      int panelHeight = getHeight();
      int srcWidth = srcImage.getWidth(null);
      int srcHeight = srcImage.getHeight(null);
      int targetWidth = srcWidth;
      int targetHeight = srcHeight;
      if (scaleLarger || srcWidth > panelWidth || srcHeight > panelHeight) {
         targetWidth = Math.round(panelHeight * (srcWidth / (float)srcHeight));
         if (targetWidth > panelWidth) {
            targetHeight = Math.round(panelWidth * (srcHeight / (float)srcWidth));
            targetWidth = panelWidth;
         } else
            targetHeight = panelHeight;
      }
      ScalingInfo info = new ScalingInfo();
      info.targetWidth = targetWidth;
      info.targetHeight = targetHeight;
      info.srcWidth = srcWidth;
      info.srcHeight = srcHeight;
      info.x = panelWidth / 2 - targetWidth / 2;
      info.y = panelHeight / 2 - targetHeight / 2;
      return info;
   }

   public void paint (Graphics g) {
      if (srcImage == null) return;

      Graphics2D g2 = (Graphics2D)g.create();
      ScalingInfo info = getScalingInfo();

      switch (scalingType) {
      case nearestNeighbor:
         scaleWithDrawImage(g2, info, RenderingHints.VALUE_INTERPOLATION_NEAREST_NEIGHBOR);
         break;
      case bilinear:
         scaleWithDrawImage(g2, info, RenderingHints.VALUE_INTERPOLATION_BILINEAR);
         break;
      case bicubic:
         scaleWithDrawImage(g2, info, RenderingHints.VALUE_INTERPOLATION_BICUBIC);
         break;
      case areaAveraging:
         scaleWithGetScaledInstance(g2, info, Image.SCALE_AREA_AVERAGING);
         break;
      case replicate:
         scaleWithGetScaledInstance(g2, info, Image.SCALE_REPLICATE);
         break;
      }
   }

   private void scaleWithGetScaledInstance (Graphics2D g2, ScalingInfo info, int hints) {
      Image srcImage = getSourceImage(info);
      Image scaledImage = srcImage.getScaledInstance(info.targetWidth, info.targetHeight, hints);
      g2.drawImage(scaledImage, info.x, info.y, null);
   }

   private void scaleWithDrawImage (Graphics2D g2, ScalingInfo info, Object hint) {
      g2.setRenderingHint(RenderingHints.KEY_INTERPOLATION, hint);

      int tempDestWidth = info.srcWidth / 2, tempDestHeight = info.srcHeight / 2;
      if (tempDestWidth < info.targetWidth) tempDestWidth = info.targetWidth;
      if (tempDestHeight < info.targetHeight) tempDestHeight = info.targetHeight;

      Image srcImage = getSourceImage(info);

      // If not doing multipass or multipass only needs a single pass, just scale it once directly to the panel surface.
      if (multiPassType == MultipassType.none || (tempDestWidth == info.targetWidth && tempDestHeight == info.targetHeight)) {
         g2.drawImage(srcImage, info.x, info.y, info.targetWidth, info.targetHeight, null);
         return;
      }

      BufferedImage tempImage = new BufferedImage(tempDestWidth, tempDestHeight, BufferedImage.TYPE_INT_RGB);
      Graphics2D g2temp = tempImage.createGraphics();
      switch (multiPassType) {
      case nearestNeighbor:
         g2temp.setRenderingHint(RenderingHints.KEY_INTERPOLATION, RenderingHints.VALUE_INTERPOLATION_NEAREST_NEIGHBOR);
         break;
      case bilinear:
         g2temp.setRenderingHint(RenderingHints.KEY_INTERPOLATION, RenderingHints.VALUE_INTERPOLATION_BILINEAR);
         break;
      case bicubic:
         g2temp.setRenderingHint(RenderingHints.KEY_INTERPOLATION, RenderingHints.VALUE_INTERPOLATION_BICUBIC);
         break;
      }
      // Render first pass from image to temp.
      g2temp.drawImage(srcImage, 0, 0, tempDestWidth, tempDestHeight, null);
      // Render passes between the first and last pass.
      int tempSrcWidth = tempDestWidth;
      int tempSrcHeight = tempDestHeight;
      while (true) {
         if (tempDestWidth > info.targetWidth) {
            tempDestWidth = tempDestWidth / 2;
            if (tempDestWidth < info.targetWidth) tempDestWidth = info.targetWidth;
         }

         if (tempDestHeight > info.targetHeight) {
            tempDestHeight = tempDestHeight / 2;
            if (tempDestHeight < info.targetHeight) tempDestHeight = info.targetHeight;
         }

         if (tempDestWidth == info.targetWidth && tempDestHeight == info.targetHeight) break;

         g2temp.drawImage(tempImage, 0, 0, tempDestWidth, tempDestHeight, 0, 0, tempSrcWidth, tempSrcHeight, null);

         tempSrcWidth = tempDestWidth;
         tempSrcHeight = tempDestHeight;
      }
      g2temp.dispose();
      // Render last pass from temp to panel surface.
      g2.setRenderingHint(RenderingHints.KEY_INTERPOLATION, hint);
      g2.drawImage(tempImage, info.x, info.y, info.x + info.targetWidth, info.y + info.targetHeight, 0, 0, tempSrcWidth,
         tempSrcHeight, null);
   }

   private Image getSourceImage (ScalingInfo info) {
      if (!blur || srcImageBlurred == null) return srcImage;
      if (info.srcWidth / 2 < info.targetWidth || info.srcHeight / 2 < info.targetHeight) return srcImage;
      return srcImageBlurred;
   }

   static private class ScalingInfo {
      public int targetWidth;
      public int targetHeight;
      public int srcWidth;
      public int srcHeight;
      public int x;
      public int y;
   }

   static public enum MultipassType {
      none, nearestNeighbor, bilinear, bicubic
   }

   static public enum ScalingType {
      nearestNeighbor, replicate, bilinear, bicubic, areaAveraging
   }
}


and some usefull utility

Code: Select all

package arcane.ui.util;

import java.awt.Graphics2D;
import java.awt.Image;
import java.awt.image.BufferedImage;
import java.awt.image.ConvolveOp;
import java.awt.image.Kernel;
import java.io.IOException;
import java.io.InputStream;

import javax.imageio.ImageIO;

public class ImageUtil {
   static public BufferedImage getImage (InputStream stream) throws IOException {
      Image tempImage = ImageIO.read(stream);
      BufferedImage image = new BufferedImage(tempImage.getWidth(null), tempImage.getHeight(null), BufferedImage.TYPE_INT_RGB);
      Graphics2D g2 = image.createGraphics();
      g2.drawImage(tempImage, 0, 0, null);
      g2.dispose();
      return image;
   }

   static public BufferedImage getBlurredImage (BufferedImage image, int radius, float intensity) {
      float weight = intensity / (radius * radius);
      float[] elements = new float[radius * radius];
      for (int i = 0, n = radius * radius; i < n; i++)
         elements[i] = weight;
      ConvolveOp blurOp = new ConvolveOp(new Kernel(radius, radius, elements));
      return blurOp.filter(image, null);
   }
}


Comes down to

Code: Select all

BufferedImage srcImage = ImageUtil.getImage(stream); // only need stream
BufferedImage srcImageBlurred = ImageUtil.getBlurredImage(srcImage, 3, 1.0f); // get a blured image

//initialization only needed once, or if you change settings
ScaledImagePanel cardImagePanel = new ScaledImagePanel(); // < our JPanel
cardImagePanel.setScalingBlur(true); //use blured image if scaling down more than 50%
cardImagePanel.setScaleLarger(true); //upscale if needed true
cardImagePanel.setScalingType(ScalingType.bicubic); // type of scaling bicubic has good quality / speed ratio
// initialization end
cardImagePanel.setImage(srcImage, srcImageBlurred);
cardImagePanel.repaint();


[DennisBergkamp]

Thanks a lot for the help guys, it's very much appreciated!
I think I got Snacko's code for displaying pictures working... however, after some tests I still see the memory use grow considerably (basically the same magnitude as it was originally).
This could very well be because I'm doing things wrong... I'm not really sure how to use the inputStream. The code looks like this currently:

Code: Select all

 public void updateCardDetailPicture(Card c)
    {
       if (c.getImageName().equals(current_picture) /*&& !c.isBasicLand()*/)
          return;
   
   current_picture = c.getImageName();
       
       BufferedInputStream stream = (BufferedInputStream) GuiDisplayUtil.getPictureStream(c);
       BufferedImage srcImage;
      try {
         srcImage = arcane.ui.util.ImageUtil.getImage(stream);
         BufferedImage srcImageBlurred = arcane.ui.util.ImageUtil.getBlurredImage(srcImage, 3, 1.0f); // get a blurred image
         
         cardImagePanel.setImage(srcImage, srcImageBlurred);
         cardImagePanel.repaint();
         
      } catch (IOException e) {
         // TODO Auto-generated catch block
         e.printStackTrace();
      } // only need stream
    }//updateCardDetail()

And GuiDisplayUtil.getPictureStream(Card c) :

Code: Select all

 public static InputStream getPictureStream(Card c)
    {
       String suffix = ".jpg";
        String filename = "";
        if(!c.isFaceDown()) {
            String basicLandSuffix = "";
            if(c.isBasicLand()) {
                if(c.getRandomPicture() != 0) {
                   basicLandSuffix = Integer.toString(c.getRandomPicture());
                   //c.setImageName(c.getImageName() + basicLandSuffix);
                }

            }
            filename = cleanString(c.getImageName())+ basicLandSuffix + suffix;
        } else filename = "morph" + suffix;
       
        String loc = "";
        if (!c.isToken())
           loc = IMAGE_BASE;
        else
           loc = IMAGE_TOKEN;
       
        String fileString = ForgeProps.getFile(loc)+"\\" +filename;
       
        try {
         BufferedInputStream is = new BufferedInputStream(new FileInputStream(fileString));
         return is;
      } catch (FileNotFoundException e) {
         // TODO Auto-generated catch block
         e.printStackTrace();
      }
      return null;
    }

Is there anything wrong with this approach right off the bat? It seems like I'm creating new InputStreams everytime a picture gets updated, which probably would also create a memory leak... is there some way to do this differently?

[silly freak]

this really looks like reading from a file every time a card is displayed. what you need is a cache between the card and the image.
basically, generate a string key from the card (filename may be a good idea), then look up the image in the cache (a Map<String, SoftReference<Image>>). If the cache doesn't contain it, or the reference is cleared, look up the file and store the image in the cache.

[Snacko]

Actually how often do you call updateCardDetailPicture. You only need to set the picture once per panel.

About Map<String, SoftReference<Image>>, it might be easier to import Google Collections
Then making and managing the cache is as easy as

Code: Select all

//one way
ConcurrentMap<String, BufferedImage> imageCache = new MapMaker().softValues().makeMap();

BufferedImage srcImage = null;
if(imageCache.containsKey(card))
srcImage = imageCache.get(card);
else {
InputStream stream = new URL(url).openStream();
srcImage = ImageIO.read(stream);
imageCache.put(card, srcImage);
}

//other way
ConcurrentMap<String, BufferedImage> imageCache = new MapMaker().softValues().makeComputingMap(
           new Function<String, Image>() {
             public Image apply(String name) {
               return loadMyImage(name);
             }
           });
//then you can do
BufferedImage srcImage = imageCache.get(name); // < always gets image, if it doesn't exist create and return it


In the above the map automatically wraps all values into soft references and they're garbage collected when the memory runs low and they're not used by any hard references.

[DennisBergkamp]

I will try this... thanks guys!
A few weeks ago I was messing around with caching these too, but gave up after a bit... I think I stored them in a HashMap of <String, JPanel> though, guess that doesn't work

[DennisBergkamp]

When using the Google Libraries, I'm running into some really weird issues:

The Import javax.annotation.Nullable; cannot be resolved.

I downloaded JSR 305, which has this class and added it to my project... but the error still persists.

[DennisBergkamp]

Never mind, it works now.
However, I'm still getting a steady increase in memory use when testing this new way, so I'm not sure if this actually fixes the memory leak.