2

I am experiencing a behavior in VirtualFlow and am not sure if it is a memory leak or if it is designed that way.

In a TableView, I expect that when the TableRow instances are not used, they should be able to be garbage collected. However, when I follow these steps:

Step #1: I opened a stage with a TableView with no data in it. I checked the memory for live instances of TableRow and see no instances (which is what I expect).

Step #2: I populated the TableView with some data, which generates the TableRows as per the required space. In my scenario, let's say it generated 38 instances of TableRow objects.

Step #3: I cleared the items in the TableView and the placeholder message is displayed. When I check for the TableRow instances (after performing multiple GCs), it still has all 38 references of TableRows strongly held in VirtualFlow's cells ArrayLinkedlist, as shown in the following image:

enter image description here

Is this a memory leak or is it expected behavior that has already been considered?

Here is the code I used to check the above scenario:

import javafx.application.Application;
import javafx.beans.property.IntegerProperty;
import javafx.beans.property.SimpleIntegerProperty;
import javafx.beans.property.SimpleStringProperty;
import javafx.beans.property.StringProperty;
import javafx.collections.FXCollections;
import javafx.collections.ObservableList;
import javafx.geometry.Insets;
import javafx.scene.Scene;
import javafx.scene.control.Button;
import javafx.scene.control.TableColumn;
import javafx.scene.control.TableView;
import javafx.scene.layout.HBox;
import javafx.scene.layout.Priority;
import javafx.scene.layout.VBox;
import javafx.stage.Stage;

public class TableViewMemoryLeakDemo extends Application {

    public static void main(String... a) {
        Application.launch(a);
    }

    @Override
    public void start(final Stage primaryStage) {
        final ObservableList<TableDataObj> items = FXCollections.observableArrayList();
        final TableView<TableDataObj> table = buildTable();
        table.setItems(items);

        Button clear = new Button("Clear All");
        clear.setOnAction(e -> {
            items.clear();
        });

        Button add = new Button("Add 50 Rows");
        add.setOnAction(e -> {
            for (int i = 0 + 1; i < 50; i++) {
                final String firstName = "First Name " + i;
                final String lastName = "Last Name " + i;
                final String city = "City " + i;
                items.add(new TableDataObj(i, firstName, lastName, city));
            }
        });
        HBox row = new HBox(add, clear);
        row.setSpacing(10);

        final VBox root = new VBox(row, table);
        root.setSpacing(10);
        root.setPadding(new Insets(10));
        VBox.setVgrow(table, Priority.ALWAYS);

        final Scene sc = new Scene(root, 1200, 950);
        primaryStage.setScene(sc);
        primaryStage.setTitle("TableView MemoryLeak Demo " + System.getProperty("javafx.runtime.version"));
        primaryStage.show();
    }

    private TableView<TableDataObj> buildTable() {
        final TableColumn<TableDataObj, Integer> idCol = new TableColumn<>();
        idCol.setText("Id");
        idCol.setCellValueFactory(param -> param.getValue().idProperty().asObject());
        idCol.setPrefWidth(100);

        final TableColumn<TableDataObj, String> fnCol = new TableColumn<>();
        fnCol.setText("First Name");
        fnCol.setCellValueFactory(param -> param.getValue().firstNameProperty());
        fnCol.setPrefWidth(150);

        final TableColumn<TableDataObj, String> lnCol = new TableColumn<>();
        lnCol.setText("Last Name");
        lnCol.setCellValueFactory(param -> param.getValue().lastNameProperty());
        lnCol.setPrefWidth(150);

        final TableColumn<TableDataObj, String> cityCol = new TableColumn<>();
        cityCol.setText("City");
        cityCol.setCellValueFactory(param -> param.getValue().cityProperty());
        cityCol.setPrefWidth(150);

        final TableView<TableDataObj> tableView = new TableView<>();
        tableView.getColumns().addAll(idCol, fnCol, lnCol, cityCol);
        return tableView;
    }

    /**
     * Data object.
     */
    class TableDataObj {
        private final IntegerProperty id = new SimpleIntegerProperty();
        private final StringProperty firstName = new SimpleStringProperty();
        private final StringProperty lastName = new SimpleStringProperty();
        private final StringProperty city = new SimpleStringProperty();

        public TableDataObj(final int i, final String fn, final String ln, final String cty) {
            setId(i);
            setFirstName(fn);
            setLastName(ln);
            setCity(cty);
        }

        public StringProperty cityProperty() {
            return city;
        }

        public StringProperty firstNameProperty() {
            return firstName;
        }

        public int getId() {
            return id.get();
        }

        public IntegerProperty idProperty() {
            return id;
        }

        public StringProperty lastNameProperty() {
            return lastName;
        }

        public void setCity(final String city1) {
            city.set(city1);
        }

        public void setFirstName(final String firstName1) {
            firstName.set(firstName1);
        }

        public void setId(final int idA) {
            id.set(idA);
        }

        public void setLastName(final String lastName1) {
            lastName.set(lastName1);
        }
    }
}
Sai Dandem
  • 8,229
  • 11
  • 26
  • I see something similar, but the maximum number of `TableRow` instances is limited by the largest view size; try minimizing and maximizing the `Stage` to see the effect. – trashgod Jun 16 '23 at 07:23
  • 4
    Surely this is the expected behavior? Isn’t the whole point that the cell/row instances are cached so they can be reused when needed? – James_D Jun 16 '23 at 10:23
  • @James_D, I am under the impression that, it should be weakly held in a List, so that it can free up the objects when they are not required. – Sai Dandem Jun 16 '23 at 11:47
  • @kleopatra, I checked against the latest ea. it is still the same. – Sai Dandem Jun 16 '23 at 11:49
  • 5
    @SaiDandem Can you cite that? At any rate, I don't think this qualifies as a memory leak. What happens when you re-populate the table? If it creates 38 new row instances and leaves the original 38 in memory unused, then that's a memory leak. If it reuses the existing 38 (which is what I suspect it does), then that's not a memory leak: it's an implementation choice. – James_D Jun 16 '23 at 13:04
  • @James_D Thanks for the inputs. Initially I am bit skeptical about this because I have multiple tables (more than 25) which mostly be empty but renders the data occasionally. So thought of that why all these rows are in memory. If that is the intended behavior, than I have to accept it :) – Sai Dandem Jun 19 '23 at 00:00

1 Answers1

5

As noted in comments above and illustrated here, many JavaFX view components provide for flyweight rendering: a factory method is invoked to create a small number of instances that can be reused to render visible content. TableView typically requires only one TableRow per visible (or partially visible) row. Maximizing the enclosing stage illustrates the effect. As @James_D observes, "Surely this is the expected behavior?"

In normal use, the underlying data would be updated in situ, as shown in your example. Should the need arise in the larger context of VirtualFlow, the variation below adds a Refresh button to examine the effect of invoking refresh() on the table. This allows unused table rendering apparatus to be collected in a future pass. As a test, enlarge the stage, and note the larger number of rows. Shrink the stage, click Refresh and request garbage collection. Note the reduced number of TableRow instances.


import javafx.application.Application;
import javafx.beans.property.IntegerProperty;
import javafx.beans.property.SimpleIntegerProperty;
import javafx.beans.property.SimpleStringProperty;
import javafx.beans.property.StringProperty;
import javafx.collections.FXCollections;
import javafx.collections.ObservableList;
import javafx.geometry.Insets;
import javafx.scene.Scene;
import javafx.scene.control.Button;
import javafx.scene.control.TableColumn;
import javafx.scene.control.TableView;
import javafx.scene.layout.HBox;
import javafx.scene.layout.Priority;
import javafx.scene.layout.VBox;
import javafx.stage.Stage;

/**
 * @see https://stackoverflow.com/q/76487091/230513
 */
public class TableViewLeak extends Application {

    public static void main(String... a) {
        Application.launch(a);
    }

    @Override
    public void start(final Stage primaryStage) {
        final ObservableList<TableDataObj> items = FXCollections.observableArrayList();
        final TableView<TableDataObj> table = buildTable();
        table.setItems(items);

        Button add = new Button("Add 50 Rows");
        add.setOnAction(e -> {
            for (int i = 0 + 1; i < 50; i++) {
                final String firstName = "First Name " + i;
                final String lastName = "Last Name " + i;
                final String city = "City " + i;
                items.add(new TableDataObj(i, firstName, lastName, city));
            }
        });
        Button clear = new Button("Clear All");
        clear.setOnAction(e -> {
            items.clear();
        });
        Button remove = new Button("Refresh");
        remove.setOnAction((e) -> {
            table.refresh();
        });

        HBox row = new HBox(add, clear, remove);
        row.setSpacing(10);
        final VBox root = new VBox(row, table);
        root.setSpacing(10);
        root.setPadding(new Insets(10));
        VBox.setVgrow(table, Priority.ALWAYS);

        final Scene sc = new Scene(root);
        primaryStage.setScene(sc);
        primaryStage.setTitle("TableView MemoryLeak Demo "
            + System.getProperty("javafx.runtime.version"));
        primaryStage.show();
    }

    private TableView<TableDataObj> buildTable() {
        final TableColumn<TableDataObj, Integer> idCol = new TableColumn<>();
        idCol.setText("Id");
        idCol.setCellValueFactory(param -> param.getValue().idProperty().asObject());
        idCol.setPrefWidth(100);

        final TableColumn<TableDataObj, String> fnCol = new TableColumn<>();
        fnCol.setText("First Name");
        fnCol.setCellValueFactory(param -> param.getValue().firstNameProperty());
        fnCol.setPrefWidth(150);

        final TableColumn<TableDataObj, String> lnCol = new TableColumn<>();
        lnCol.setText("Last Name");
        lnCol.setCellValueFactory(param -> param.getValue().lastNameProperty());
        lnCol.setPrefWidth(150);

        final TableColumn<TableDataObj, String> cityCol = new TableColumn<>();
        cityCol.setText("City");
        cityCol.setCellValueFactory(param -> param.getValue().cityProperty());
        cityCol.setPrefWidth(150);

        final TableView<TableDataObj> tableView = new TableView<>();
        tableView.getColumns().addAll(idCol, fnCol, lnCol, cityCol);
        return tableView;
    }

    /**
     * Data object.
     */
    private static final class TableDataObj {

        private final IntegerProperty id = new SimpleIntegerProperty();
        private final StringProperty firstName = new SimpleStringProperty();
        private final StringProperty lastName = new SimpleStringProperty();
        private final StringProperty city = new SimpleStringProperty();

        public TableDataObj(final int i, final String fn, final String ln, final String cty) {
            setId(i);
            setFirstName(fn);
            setLastName(ln);
            setCity(cty);
        }

        public StringProperty cityProperty() {
            return city;
        }

        public StringProperty firstNameProperty() {
            return firstName;
        }

        public int getId() {
            return id.get();
        }

        public IntegerProperty idProperty() {
            return id;
        }

        public StringProperty lastNameProperty() {
            return lastName;
        }

        public void setCity(final String city1) {
            city.set(city1);
        }

        public void setFirstName(final String firstName1) {
            firstName.set(firstName1);
        }

        public void setId(final int idA) {
            id.set(idA);
        }

        public void setLastName(final String lastName1) {
            lastName.set(lastName1);
        }
    }
}
trashgod
  • 203,806
  • 29
  • 246
  • 1,045