SortedList hanging on to references & preventing GC

[Cormac Redmond]

Hi,

Thanks. I've opened a draft PR: https://github.com/openjdk/jfx/pull/2000 (first
time, be kind!)...

As requested per https://github.com/openjdk/jfx/blob/master/CONTRIBUTING.md,
the tests I added do fail without the fixes in place, around both the
"sorted" array and tempElement, and pass with them in place.

Aside from that, a lot of manual testing has not found any issues also.




Kind REgards,
Cormac

On Fri, 5 Dec 2025 at 04:20, John Hendrikx <john.hendrikx at gmail.com> wrote:

>
> On 04/12/2025 21:21, Cormac Redmond wrote:
>
> Hi John,
>
> Thanks for the prompt response. Yes, I looked into the code afterwards,
> and I can see that it is just fairly oblivious to the waste. I was looking
> to override with nulls myself, but there were too many privates.
>
> The only solution (without re-writing my own impl) is to re-create the
> SortedList every time the dataset is smaller, or to create some sort of DTO
> that does not link to the original real objects in any way (or maybe use
> WeakReferences). In my case, I could have 10,000 rows where each item's
> underlying memory is around 500KB on the heap. That's instantly 5GB I
> cannot re-claim, or example.
>
> That code with the "bug" *mostly* hasn't changed in 10+ years. Can it be
> fixed? It's such a fundamental class, adding workarounds (everywhere,
> forever) would seem a shame.
>
> It definitely can be fixed.  Nothing should be relying on the current
> behavior, as it is not specified to hold on to stale references.
>
> You could try submit a PR with a fix, or wait until this becomes a
> priority.  I myself may also do a fix when I have time, as it is pretty low
> hanging fruit and self contained.
>
> You can "test" a fixed implementation by just making a copy of the class.
> If you're not in a modular environment, you can even replace the class (in
> the same package it is in) with a fixed version until FX fixes it.  This is
> how I usually test and fix (deep) internal problems without having to build
> FX locally at all.
>
> --John
>
>
>
>
>
>
> Kind Regards,
> Cormac
>
> On Thu, 4 Dec 2025 at 20:01, John Hendrikx <john.hendrikx at gmail.com>
> wrote:
>
>> This looks like a classic problem where unused elements in an array are
>> not nulled.  Looking at the code that updates the `size` field, there are a
>> few code paths that are not setting unused elements to null.  I also saw no
>> code that ever shrinks the arrays.  So it looks this implementation is only
>> half finished.
>>
>> So, yes, I'd say this is a bug.
>>
>> I'm going to guess there's a very good reason for this behaviour.
>>
>> Time pressure and insufficient testing.
>>
>> --John
>> On 04/12/2025 19:40, Cormac Redmond wrote:
>>
>> Hi,
>>
>> I've traced a memory issue back to a SortedList (surprisingly), where
>> it's hanging on to objects that could/should have been GC'd.
>>
>> SortedList's internal arrays will only grow (but not shrink) in line with
>> the source ObservableList (I'm sure there are reasons for this). So even
>> when your source ObservableList shrinks, SortedList is hanging on to
>> references to objects the source list once contained, even though they're
>> completely removed from the source list.
>>
>> This leads to a substantial waste of memory, especially when just one
>> momentarily large dataset leads to a permanent spike in unnecessary memory
>> usage for the remainder for the application's lifetime. I'm sure I'm not
>> the first to raise this or ask about it & I'm going to guess there's a very
>> good reason for this behaviour. But could someone explain this? I would
>> have thought SortedList should/could remain as lean as the source list.
>> FilteredList is somewhat similar, except it stores an array of ints
>> (indexes), so less of a memory hit, but still pointless nonetheless.
>>
>> Example GIF + code below: setup a typical sortable table (so,
>> ObservableList + FilteredList + SortableList), where the print button shows
>> that when you add a lot of data, and remove it, SortedList retains
>> references to all of the old objects. Some reflection is used to print that
>> information.
>>
>> [image: sorted_list_mem.gif]
>>
>>
>> Code to reproduce:
>>
>> public class SortedListMemWasteDemo extends Application {
>>
>>     record PotentialLargeData(String info) {}
>>
>>     public static void main(String[] args) {
>>         launch(args);
>>     }
>>
>>     @Override
>>     public void start(Stage primaryStage) {
>>         ObservableList<PotentialLargeData> masterData =
>> FXCollections.observableArrayList(
>>                 new PotentialLargeData("Initial info 1"),
>>                 new PotentialLargeData("Initial info 2"));
>>
>>         TableColumn<PotentialLargeData, String> col = new
>> TableColumn<>("Info");
>>         col.setCellValueFactory(cellData -> new
>> SimpleStringProperty(cellData.getValue().info()));
>>
>>         TableView<PotentialLargeData> table = new TableView<>();
>>         table.getColumns().add(col);
>>
>>         FilteredList<PotentialLargeData> filteredData = new
>> FilteredList<>(masterData, item -> true);
>>         SortedList<PotentialLargeData> sortedData = new
>> SortedList<>(filteredData);
>>         sortedData.comparatorProperty().bind(table.comparatorProperty());
>>         table.setItems(sortedData);
>>
>>         Button loadManyBtn = new Button("Add 10,000 items");
>>         loadManyBtn.setOnAction(e -> {
>>             System.out.println("Adding 10000 items to master data");
>>             masterData.clear();
>>             for (int i = 0; i < 10000; i++) masterData.add(new
>> PotentialLargeData("Info item " + i));
>>         });
>>
>>         Button reduceBtn = new Button("Reduce to 2");
>>         reduceBtn.setOnAction(e -> {
>>             System.out.println("Reducing master data size to 2");
>>             masterData.setAll(new PotentialLargeData("New info 1"), new
>> PotentialLargeData("New info 2"));
>>         });
>>
>>         Button printBtn = new Button("Print interal sizes");
>>         printBtn.setOnAction(e -> {
>>             try {
>>                 System.out.println("Items the user sees: " +
>> sortedData.size());
>>                 int[] filtered = (int[]) getFieldValue(filteredData,
>> "filtered");
>>                 System.out.println("FilteredList.filtered length: " +
>> filtered.length);
>>                 // This is a hidden and significant waste of memory
>>                 Object[] sorted = (Object[]) getFieldValue(sortedData,
>> "sorted");
>>                 System.out.println("SortedList.sorted length: " +
>> sorted.length);
>>             } catch (Exception ex) {
>>                 throw new RuntimeException(ex);
>>             }
>>         });
>>
>>         primaryStage.setScene(new Scene(new VBox(table, loadManyBtn,
>> reduceBtn, printBtn), 600, 400));
>>         primaryStage.show();
>>     }
>>
>>     private static Object getFieldValue(Object object, String fieldName)
>> throws Exception {
>>         Field field = object.getClass().getDeclaredField(fieldName);
>>         field.setAccessible(true);
>>         return field.get(object);
>>     }
>> }
>>
>>
>> Kind Regards,
>> Cormac
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/openjfx-dev/attachments/20251207/381eb032/attachment-0001.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sorted_list_mem.gif
Type: image/gif
Size: 128014 bytes
Desc: not available
URL: <https://mail.openjdk.org/pipermail/openjfx-dev/attachments/20251207/381eb032/sorted_list_mem-0001.gif>