javafxmemory-leakstreetableviewjava-memory-leaks

JavaFX, TreeTableView, RowFactory, and memory leak


I am using the JavaFX TreeTableView to display sales orders. Parent rows contain the "master" record, and the child rows contain information about the order, including line items for each product ordered. I am using a RowFactory to highlight the rows in the table based on its current status. The code for the RowFactory begins with this code:

        currentOrderTree.setRowFactory(new     Callback<TreeTableView<MmTicket>, TreeTableRow<MmTicket>>() {
            @Override
            public TreeTableRow<MmTicket> call(TreeTableView<MmTicket> p) {
                final TreeTableRow<MmTicket> row = new TreeTableRow<MmTicket>() {
                    @Override
                    protected void updateItem(MmTicket order, boolean empty) {
                        super.updateItem(order, empty);
                        if (order != null) {
                System.out.println("Calling rowFactory method for the " + ++rowcall + " time.");
                            if (packingListPendingList.containsKey(order.getSono())) {
                                if (!getStyleClass().contains("pending")) {
                                    getStyleClass().remove("working");
                                    getStyleClass().remove("fulfilled");
                                    getStyleClass().remove("completed");
                                    getStyleClass().add("pending");
                                    getStyleClass().remove("shipped");
                                }
....

As you can see, the Callback returns a new TreeTableRow each time the row factory is called. According to the JavaFX 8.0 documentation, it is the system's responsibility to manage the creation of rows, reusing them when appropriate. The System.out.println call documents the number of times the method is called. Scrolling the table up and down a number of times, as well as data refreshes caused by updates to the sales database, result in this method being called tens of thousands of times in a relatively short period of time. My memory usage in the program continues to grow as the TreeTableView is used extensively by the end-users.

Profiling the application shows that there are very large amounts of memory being used by HashTable, and PsuedoClass (css stuff, I believe) and things such as byte[] and char[] and int[]. I have tried different combinations of heap sizes and garbage collectors, but the end result is always the same, in that the application eventually runs out of heap space.

Searching for answers has shown that there are some who are experiencing this problem, and I have found that if I don't do any of the row highlighting, thereby not calling the row factory, the program is much better at managing memory. Anyone have any insights as to what might be causing this problem, or possible solutions?


Solution

  • If you track the number of row objects being created, you'll find there are not many. Clearly, since updateItem(...) is being called so many times there is extensive reuse happening.

    One thing to be aware of is that styleClass is implemented as a list, which of course allows duplicates. So you need to be very careful about managing its content in a method such as updateItem(...) when you have no control over when the method is called and what parameters are passed to it. Specifically, you probably want to make sure there is no possible sequence of calls to updateItem(...) that would result in the same value being added to the style class multiple times. This includes invocations with order=null. (What happens, e.g. in your case if the same cell alternates between being used for an empty cell, with order=null and a "pending" cell?) It looks like you are checking for this by testing (if (! getStyleClass().contains("pending"))) but there are some corner cases that can be easy to overlook.

    Try adding getStyleClass().size() to your debugging output, to see if it is growing excessively.

    A cleaner solution anyway might be to use CSS PseudoClasses instead of the style class. You can do something along the lines of

    PseudoClass pending = PseudoClass.getPseudoClass("pending");
    PseudoClass working = PseudoClass.getPseudoClass("working");
    PseudoClass fulfilled = PseudoClass.getPseudoClass("fulfilled");
    PseudoClass completed = PseudoClass.getPseudoClass("completed");
    PseudoClass shipped = PseudoClass.getPseudoClass("shipped");
    
    
    @Override
    protected void updateItem(MmTicket order, boolean empty) {
        super.updateItem(order, empty);
    
        pseudoClassStateChanged(pending, order != null && packingPendingList.containsKey(order.getSono()));
        pseudoClassStateChanged(working, order != null && packingWorkingList.containsKey(order.getSono()));
        pseudoClassStateChanged(fulfilled, order != null && packingFulfilledList.containsKey(order.getSono()));
        pseudoClassStateChanged(completed, order != null && packingCompletedList.containsKey(order.getSono()));
        pseudoClassStateChanged(shipped, order != null && packingShippedList.containsKey(order.getSono()));
    }
    

    (or some appropriate logic). PseudoClasses only have two states (set or not set) and so this avoids all the issues with managing the style class list.

    Your CSS in this case would look like

    .table-row-cell {
        /* basic styles... */
    }
    
    .table-row-cell:pending {
        /* styles specific to pending items */
    }
    
    .table-row-cell:working {
        /* styles specific to working items */
    }
    
    /* etc etc */
    

    It probably doesn't apply in your use case, but you can also match nodes with multiple PseudoClasses set with

    .table-row-cell:pending:working { /* ... */ }
    

    If you do want to stick with style classes, first make sure you handle the null case. You might want something like

    List<String> allStyles = Arrays.asList("pending", "working", "fulfilled", "completed", "shipped");
    
    @Override
    protected void updateItem(MmTicket order, boolean empty) {
    
        super.updateItem(order, empty);
    
        if (order == null) {
            getStyleClass().removeAll(allStyles);
        } else {
           // ...
        }
    }
    

    and instead of

    getStyleClass().remove("pending");
    

    which removes the first occurrence of "pending" in the style class list, consider

    getStyleClass().removeAll(Collections.singletonList("pending"));
    

    as the removeAll(Collection) method removes all elements which are contained in the supplied collection.

    Again, this may not be the cause of the issue, but it's very easy to let the style class list grow indefinitely if you manipulate it in the updateItem() method.