Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8098191

TreeView creates new cells every time root is changed

    XMLWordPrintable

    Details

      Description

      I've found that in certain situations, TreeView will aggresively create new cells every time the root is changed. I've narrowed it down to the provided standalone program.

      This is problematic for two reasons:

      1) Performance, creating Cells can be time consuming, complex cells can take upto 15ms to create (and bind to model), and when 30 need to be created this results in noticeable delays when switching the root of a tree in response to a user action. Without the new cells being created this would be nearly instant (<100ms).

      2) Binding polution (for lack of better term) -- every cell binds itself to a model to keep up to date, in my case about 4-5 bindings per cell. The model itself is static and thus isn't recreated (and garbage collected) every now and then. This means that 'old' Cells keep their bindings to the model. After some use it is not uncommon to see one entry in the model having dozens of bindings from old, discarded Cells, which in turn donot get gc'd.

      First, I think that we should establish that binding to a model in a Cell is a good thing (if not, then that's the problem -- but then we need a different solution for keeping cells up to date).

      There is however no way to find out when a Cell is discarded, which is essential for being able to clean up the bindings that were made. Even if the TreeView is a bit more economic with the creation of cells (in this case), I still think that after a while (with a Model that never gets gc'd) many old bindings will be present (from discarded TreeCells or even from TreeCells belonging to long disposed of TreeView's).

      I think this should be a high priority as any program dealing with TreeView that uses this kind of binding in cells and has this problem with cells being recreated will run out of heap during prolonged use.

      Here is the example code, running it will slowly kill the JVM (it takes a while to die, it seems the GC is reluctant to admit that it is out of heap space...)

      package hs.javafx;
       
      import java.util.ArrayList;
      import java.util.List;
       
      import javafx.application.Application;
      import javafx.application.Platform;
      import javafx.beans.property.SimpleStringProperty;
      import javafx.beans.property.StringProperty;
      import javafx.scene.Scene;
      import javafx.scene.control.TreeCell;
      import javafx.scene.control.TreeItem;
      import javafx.scene.control.TreeView;
      import javafx.scene.layout.StackPane;
      import javafx.stage.Stage;
      import javafx.util.Callback;
       
      public class CopyOfTreeViewMemoryTest extends Application {
        private static final List<MediaItem> MODEL = new ArrayList<MediaItem>() {{
          for(int i = 0; i < 1000; i++) {
            add(new MediaItem("" + i));
          }
        }};
       
        public static void main(String[] args) {
          Application.launch(CopyOfTreeViewMemoryTest.class, args);
        }
       
        private static long createdCells = 0;
       
        @Override
        public void start(Stage primaryStage) throws Exception {
          StackPane stackPane = new StackPane();
          final TreeView<MediaItem> treeView = new TreeView();
          stackPane.getChildren().setAll(treeView);
       
          treeView.setShowRoot(false);
          treeView.setCellFactory(new Callback<TreeView<MediaItem>, TreeCell<MediaItem>>() {
            @Override
            public TreeCell<MediaItem> call(TreeView<MediaItem> param) {
              return new TreeCell<MediaItem>() {
                {
                  createdCells++;
                }
       
                @Override
                protected void updateItem(MediaItem item, boolean empty) {
                  super.updateItem(item, empty);
                  textProperty().unbind();
       
                  if(!empty) {
                    textProperty().bind(item.titleProperty());
                  }
                }
              };
            }
          });
       
          primaryStage.setScene(new Scene(stackPane));
          primaryStage.show();
       
          new Thread() {
            private int j = 0;
       
            @Override
            public void run() {
              for(int k = 0; k < Integer.MAX_VALUE - 10; k++) {
                try {
                  Thread.sleep(100);
                }
                catch(InterruptedException e) {
                }
                if(k % 10 == 0) {
                  long totalMemory = Runtime.getRuntime().totalMemory() / 1024;
                  long freeMemory = Runtime.getRuntime().freeMemory() / 1024;
                  System.out.println("Heap use: " + (totalMemory - freeMemory) + " / " + totalMemory + " kB -- created cells: " + createdCells);
                }
       
                Platform.runLater(new Runnable() {
                  @Override
                  public void run() {
                    TreeItem<MediaItem> root = new TreeItem(new MediaItem("bla"));
                    treeView.setRoot(root);
       
                    for(int i = j % 1000; i < (j % 1000) + 100; i++) {
                      root.getChildren().add(new MediaNodeTreeItem(MODEL.get(i)));
                    }
       
                    j+=100;
                  }
                });
              }
            }
          }.start();
        }
       
        private final class MediaNodeTreeItem extends TreeItem<MediaItem> {
          private MediaNodeTreeItem(MediaItem value) {
            super(value);
          }
       
          @Override
          public boolean isLeaf() {
            return true;
          }
        }
       
        public static class MediaItem {
          private final StringProperty title = new SimpleStringProperty();
       
          public MediaItem(String title) {
            this.title.set(title);
          }
       
          public StringProperty titleProperty() {
            return title;
          }
       
          public String getTitle() {
            return title.get();
          }
        }
      }

        Attachments

          Activity

            People

            Assignee:
            jgiles Jonathan Giles
            Reporter:
            jhendrikx John Hendrikx
            Votes:
            1 Vote for this issue
            Watchers:
            8 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:
              Imported: