0

I have a task executioner that has 5 outer panels, and each outer panel is a JFrame that has an innerPanel. The innerpanel is a JPanel responsible for painting an image. I'm displaying and removing many images, over and over, and I'm having very poor performance.

Can someone suggest a way to load and remove many images without using a JFrame?

I am displaying the images on different parts of the screen, and sometimes for extremely short durations 50-1500ms. Sometimes the images only partially loads and on others it does not load at all.

Main

public class Main {

    public static void main(String[] args) {

        new Main();
    }

    public Main() {
        ScheduledExecutorService scheduledExecutorService = Executors.newSingleThreadScheduledExecutor();
        scheduledExecutorService.scheduleWithFixedDelay(new ImageTask(new OuterPanel(ScreenPosition.CENTER)), 0, 3, TimeUnit.SECONDS);
        scheduledExecutorService.scheduleWithFixedDelay(new ImageTask(new OuterPanel(ScreenPosition.CENTER)), 0, 3, TimeUnit.SECONDS);
        scheduledExecutorService.scheduleWithFixedDelay(new ImageTask(new OuterPanel(ScreenPosition.CENTER)), 0, 3, TimeUnit.SECONDS);
        scheduledExecutorService.scheduleWithFixedDelay(new ImageTask(new OuterPanel(ScreenPosition.CENTER)), 0, 3, TimeUnit.SECONDS);
        scheduledExecutorService.scheduleWithFixedDelay(new ImageTask(new OuterPanel(ScreenPosition.CENTER)), 0, 3, TimeUnit.SECONDS);
    }
}

ImageTask

public class ImageTask implements Runnable {

    private OuterPanel outerPanel;
    private int messageIndex;

    public ImageTask(OuterPanel outerPanel) {
        this.outerPanel = outerPanel;
    }

    @Override
    public void run() {
        outerPanel.setMessage(imagePath);
        outerPanel.setVisible(true);
        try {
            TimeUnit.MILLISECONDS.sleep(150);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        outerPanel.setVisible(false);
    }
}

OuterPanel

public class OuterPanel extends JFrame {

    private InnerPanel innerPanel;
    public static int height = 200;
    public static int width = 200;

    public OuterPanel(ScreenPosition screenPosition) {
        initComponents();

        setFocusable(false);
        setUndecorated(true);
        setBackground(new Color(0, 0, 0, 0));
        setAlwaysOnTop(true);
        setSize(SetScreenLocation.screenSize.width, SetScreenLocation.screenSize.height);
        setFocusableWindowState(false);
        setEnabled(false);
        pack();

        setLocation(0, 0);
    }

    private void initComponents() {
        innerPanel = new InnerPanel();
        add(innerPanel);
    }

    public void setMessage(Message message) {
        ImageIcon icon = IconFetch.getInstance().getIcon(message.getImagePath());
        if (icon != null) {
            Image img = IconFetch.getInstance().getScaledImage(icon.getImage(), width, height);
            innerPanel.setImage(img);
        }
    }

}

InnerPanel

public class InnerPanel extends JPanel {
    private String message;
    private Image img;

    public InnerPanel() {
        setOpaque(false);
        setPreferredSize(new Dimension(900, 450));
    }

    @Override
    protected void paintComponent(Graphics g) {
        super.paintComponent(g);
        Graphics2D g2d = (Graphics2D) g;

        if (img != null) {
            x = (this.getWidth() - img.getWidth(null)) / 2;
            y = (this.getHeight() - img.getHeight(null)) / 2;
            g2d.drawImage(img, x, y, this);
        }
    }

    public void setImage(Image image) {
        this.img = image;
        repaint();
    }
}
traderjosh
  • 321
  • 5
  • 16
  • 2
    For better help sooner post a proper [mcve], this one lacks the method calls and the `main` method, so we can't know if you're placing the program on the EDT, if you're using your own thread or a `Swing Timer` or a `Utils Timer` – Frakcool Oct 24 '17 at 13:29
  • 1
    1) One way to get image(s) for an example (as mentioned by @Frakcool) is to hot link to images seen in [this Q&A](http://stackoverflow.com/q/19209650/418556). 2) See [The Use of Multiple JFrames, Good/Bad Practice?](http://stackoverflow.com/q/9554636/418556) – Andrew Thompson Oct 24 '17 at 14:13
  • I've seen that, and using a JFrame is a problem for many reasons. It creates a many windows stacked on top of one another, and it's causing performance issues. I also need to load multiple images repeatedly and display them on different parts of the screen, and sometimes for an extremely short period. I'm starting to think that this is not something that can be done in Java. – traderjosh Oct 24 '17 at 14:39
  • 1
    Tip: If you want to reply someone you should add `@` before their name, for example @AndrewThompson. Why are you trying to create new `JFrame` and displaying them for such short periods? I think it could be better to create a single `JFrame`, change the image and it's location on screen rather than creating new ones everytime as well as using a `Swing Timer` without `.sleep(...)` calls. – Frakcool Oct 24 '17 at 15:05
  • *"I also need to load multiple images repeatedly and display them on different parts of the screen"* - Maybe consider some kind of cache, loading images is can be time consuming. *"I'm starting to think that this is not something that can be done in Java"* - I'm pretty sure it can be, but you might need to be mindful of your resource management, re-using elements where possible, this might mean having a pool of frames which are just invisible, further reducing the overhead of object creation and destruction. You also need to be careful, as Swing is not thread safe – MadProgrammer Oct 24 '17 at 19:26
  • Also, if you're not doing anything special with the images, you could just use a `JLabel` instead of resorting to custom painting – MadProgrammer Oct 24 '17 at 19:27

1 Answers1

1

So, three things which are time costly:

  1. Loading images
  2. Creating/showing a frame for the first time
  3. Garbage collection

If you can reduce these overheads, you can increase the performance of the system

I can't see how IconFetch is working, but I would highly recommend that:

  1. You use ImageIO.read to load the image, as it won't return until the image is completely loaded
  2. You cache the result, so you're not re-loading the same image over and over again, which is costly.

This is a really simply example. You pass it a file path and it will load the image into the cache, if it's not already cached, otherwise it will return the cached value

public enum ImagePool {
    INSTANCE;

    private Map<String, Image> images;

    private ImagePool() {
        images = new HashMap<>(25);
    }

    public synchronized Image grab(String name) throws IOException {
        Image image = images.get(name);
        if (image == null) {
            image = ImageIO.read(new File(name));
            images.put(name, image);
        }
        return image;
    }

}

Getting a window onto the screen for the first time is a costly process, and you won't always know when the window is actually "visible" on the screen due to system over-heads.

To this end, you should focus on caching the windows into some kind of pool and re-using them as much as possible, for example:

public enum FramePool {
    INSTANCE;

    private int minSize = 15;
    private int maxSize = 25;

    private List<ImageFrame> avaliable;

    private FramePool() {
        avaliable = new ArrayList<>(25);

        for (int index = 0; index < minSize; index++) {
            avaliable.add(new ImageFrame());
        }
    }

    public synchronized ImageFrame grab() {
        ImageFrame frame;
        System.out.println(avaliable.size());
        if (avaliable.isEmpty()) {
            System.out.println("Make new");
            frame = new ImageFrame();
        } else {
            frame = avaliable.remove(0);
        }

        return frame;
    }

    public synchronized void release(ImageFrame frame) {
        if (avaliable.size() < maxSize) {
            avaliable.add(frame);
        } else {
            System.out.println("Destory");
            frame.dispose();
        }
    }

    public class ImageFrame extends JFrame {

        private JLabel label;
        private int timeout = 150;

        public ImageFrame() throws HeadlessException {
            label = new JLabel();
            add(label);
            setUndecorated(true);
            setBackground(new Color(0, 0, 0, 0));
            setAlwaysOnTop(true);
            setFocusableWindowState(false);
            setFocusable(false);
            pack();

            addWindowListener(new WindowAdapter() {
                @Override
                public void windowOpened(WindowEvent e) {
                    System.out.println("Show me");
                    Timer timer = new Timer(timeout, new ActionListener() {
                        @Override
                        public void actionPerformed(ActionEvent e) {
                            System.out.println("Hide me");
                            setVisible(false);
                            System.out.println("Return");
                            FramePool.INSTANCE.release(ImageFrame.this);
                        }
                    });
                    timer.start();
                }

            });
        }

        public void setTimeout(int timeout) {
            this.timeout = timeout;
        }

        public void setImage(Image image) {
            label.setIcon(new ImageIcon(image));
            pack();

            Rectangle bounds = new Rectangle(0, 0, 0, 0);

            GraphicsEnvironment ge = GraphicsEnvironment.getLocalGraphicsEnvironment();
            GraphicsDevice gd = ge.getDefaultScreenDevice();

            GraphicsConfiguration gc = gd.getDefaultConfiguration();
            bounds = gc.getBounds();

            Insets insets = Toolkit.getDefaultToolkit().getScreenInsets(gc);

            bounds.x += insets.left;
            bounds.y += insets.top;
            bounds.width -= (insets.left + insets.right);
            bounds.height -= (insets.top + insets.bottom);

            int x = (int) (Math.random() * bounds.width);
            int y = (int) (Math.random() * bounds.height);

            if (x + getWidth() > bounds.width) {
                x = bounds.width - getWidth();
                if (x < 0) {
                    x = 0;
                }
            }
            if (y + getHeight() > bounds.height) {
                y = bounds.height - getHeight();
                if (y < 0) {
                    y = 0;
                }
            }

            setLocation(bounds.x + x, bounds.y + y);
        }

    }
}

And then finally, you set up the tasks...

public class ImageTask implements Runnable {

    private String name;
    private int timeout;

    public ImageTask(String name, int timeout) {
        this.name = name;
        this.timeout = timeout;
    }

    @Override
    public void run() {
        try {
            System.out.println(this);
            FramePool.ImageFrame frame = FramePool.INSTANCE.grab();
            frame.setImage(ImagePool.INSTANCE.grab(name));
            frame.setTimeout(timeout);
            frame.setVisible(true);
        } catch (IOException ex) {
            ex.printStackTrace();
        }
    }
}

Now, I've taken a different approach. Basically, I simply pass the name of the image to the task, which then make use of the frame and image pools to build the result and display it on the screen.

For me, the benefit is isolated control. To get an image on the screen, I just need to know about the ImageTask and the image file name and nothing else.

For my testing, I load a bunch of images from a directory and displayed them.

ScheduledExecutorService scheduledExecutorService = Executors.newSingleThreadScheduledExecutor();
File[] files = path.listFiles((File pathname) -> pathname.getName().toLowerCase().endsWith(".png"));
for (File file : files) {
    scheduledExecutorService.scheduleWithFixedDelay(new ImageTask(file.getPath(), 250 + (int) (Math.random() * 1000)), 0, 3, TimeUnit.SECONDS);
}

Once all the object creation was out of the way, it ran smoothly

There are variations on the idea, for example, maybe you don't need the FramePool and could create a single instance for each ImageTask

MadProgrammer
  • 343,457
  • 22
  • 230
  • 366
  • I can confirm that this works. However, I don't see why you added a new `ImageTask` for every image. Why not just create a few tasks (3-8) and change the image inside them? I tried to alter your code to do that, but the images aren't loading again. I have roughly 50 images that I want to be displayed on 5 fixed locations, and I don't think it's a good idea to schedule 50 tasks for that. – traderjosh Oct 28 '17 at 20:45
  • *"and I don't think it's a good idea to schedule 50 tasks for that"* - Why? It's a single thread executor, so only one task will be executed at a time. Even if you used a pooled executor with 3-5 threads, I still don't see the issue – MadProgrammer Oct 28 '17 at 21:10
  • @TraderJosh "However, I don't see why you added a new ImageTask for every image. Why not just create a few tasks (3-8) and change the image inside them?"* - Because it's a lot of pain to try and make it work - trying to find out when a task has complete and then try and recycle it, much easier to have a task which does one single job. Sure, you could create a "batch" task which takes more then one image, but you're not gaining any more benefit. It seems you're trying to prematurely optimise the solution, wait till there is actually a problem – MadProgrammer Oct 28 '17 at 21:11