Slow resize of target location wizard

[ptziegler]

I think a video speaks more than a thousand words:

Screen.Recording.2024-11-29.085030.mp4

Looking at the profiler, I notice that almost all of the time is spent performing the layout on a tree widget, which is weird as this is the Maven wizard, which doesn't have a tree.

Using the debugger, I then checked the invocation chain and the layout is indeed done by the FilteredCheckboxTree, which is a PDE internal class and therefore unrelated to the Maven extension:

Am I correct to assume that this FilteredCheckboxTree is used for the "Target Content" tab? If so, then I don't believe it's intentional for a resize of the wizard to cause a relayout of the underlying editor?

[ptziegler]

Nevermind... M2E is extending the PDE internal BasePluginListPage and gets the FilteredCheckboxTree from there. So there is nothing to do on the PDE side.

[ptziegler]

it's this wizard page, to be precise:

Even if it's an internal class, perhaps it would make sense to convert this to a lazy viewer? Because refreshing a tree with 1400 elements during every resize will probably also affect the editor.

[vogella]

perhaps it would make sense to convert this to a lazy viewer

Yes of course. I assume you are interested in providing a PR?

By the way, I would like to express my gratitude of your effort to improve the Eclipse platform and PDE, basically every issue and PR I see from you is helpful.

[ptziegler]

Yes of course. I assume you are interested in providing a PR?

Of course. It's already painful with my normal workspace. If I check out everything, I end up with 8000 plugins, which is even more dreadful to work with and just for fun, I've set up a work environment with the 2024-12 staging repository and the Babel update site and the IDE becomes completely unusable...

By the way, I would like to express my gratitude of your effort to improve the Eclipse platform and PDE, basically every issue and PR I see from you is helpful.

Thanks 😄

I found the places that need to be adapted for the virtual tree. However, it turns out that the VIRTUAL flag for the CheckboxTreeViewer is effectively meaningless because of this little gem here, which forces the creation of all tree items. So unless this is addressed, I don't think it's possible to avoid this performance problem...

private void applyState(CustomHashtable checked, CustomHashtable grayed,
        Widget widget) {
    Item[] items = getChildren(widget);
    for (Item item : items) {
        if (item instanceof TreeItem) {
            Object data = item.getData();
            if (data != null) {
                TreeItem ti = (TreeItem) item;
                ti.setChecked(checked.containsKey(data));
                ti.setGrayed(grayed.containsKey(data));
            }
        }
        applyState(checked, grayed, item);
    }
}

[ptziegler]

I've played around with the current implementation and as far as I can tell, the underlying problem is the usage of a ContainerCheckedTreeViewer instead of CheckboxTableViewer, to represent a one-dimensional list of elements. This is because the ContainerCheckedTreeViewer needs to create the entire tree to determine the checked/grayed state and therefore can't be used with virtual trees.

As seen in 4c1edad, there used to be a TableViewer, but it was changed in order to add a filter to the viewer and to re-use the FilteredTree class provided by the platform.

My suggestion: Create a FilteredTable class and switch back to a table viewer. Edit: Or simply re-use the FilteredList...

[vogella]

We don't have a FilteredTable? Yes, we should add it. Would be great to have the option in this new class to enable/ disable the filter at runtime similar to the E4 FilteredTree

[ptziegler]

There's the FilteredList which works similar, I think? But I have to double-check.

[vogella]

IMHO every view which shows a table or a tree and does not allow to filter is bad UX.