Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes #118 - [Performance] Excel-like filter for huge data sets #119

Merged
merged 2 commits into from
Sep 30, 2024
Merged

Fixes #118 - [Performance] Excel-like filter for huge data sets #119

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3e8bf12
Fixes #118 - [Performance] Excel-like filter for huge data sets
fipro78 Sep 27, 2024
adcb22a
Fixes #118 - [Performance] Excel-like filter for huge data sets
fipro78 Sep 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 13 additions & 14 deletions ...table.core/src/org/eclipse/nebula/widgets/nattable/filterrow/combobox/FilterNatCombo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2013, 2023 Dirk Fauth and others.
* Copyright (c) 2013, 2024 Dirk Fauth and others.
*
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
Expand All @@ -17,6 +17,7 @@
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.StringJoiner;

import org.eclipse.jface.viewers.ArrayContentProvider;
import org.eclipse.jface.viewers.CheckboxTableViewer;
Expand Down Expand Up @@ -568,7 +569,7 @@ public int getSelectionIndex() {
if (!getDropdownTable().isDisposed()) {
return getDropdownTable().getSelectionIndex();
} else if (this.filterText != null && this.filterText.length() > 0) {
return this.itemList.indexOf(this.filterText);
return getItemIndex(this.filterText);
}
return -1;
}
Expand Down Expand Up @@ -845,22 +846,20 @@ public void select(int[] indeces) {
@Override
protected String getTransformedText(String[] values) {
String result = ""; //$NON-NLS-1$
if (this.multiselect) {
for (int i = 0; i < values.length; i++) {
String selection = values[i];
result += selection;
if ((i + 1) < values.length) {
result += this.multiselectValueSeparator;
}
}
if (values.length > 0) {
// if at least one value was selected, add the prefix and suffix
// we check the values array instead of the result length because
// there can be also an empty String be selected
if (values.length > 0) {
result = this.multiselectTextPrefix + result + this.multiselectTextSuffix;
if (this.multiselect) {
StringJoiner joiner = new StringJoiner(
this.multiselectValueSeparator,
this.multiselectTextPrefix,
this.multiselectTextSuffix);
Arrays.stream(values).forEach(joiner::add);
result = joiner.toString();
} else {
result = values[0];
}
} else if (values.length > 0) {
result = values[0];
}
return result;
}
Expand Down
4 changes: 2 additions & 2 deletions ...c/org/eclipse/nebula/widgets/nattable/filterrow/combobox/FilterRowComboBoxCellEditor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
*******************************************************************************/
package org.eclipse.nebula.widgets.nattable.filterrow.combobox;

import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -264,7 +264,7 @@ && getComboBoxDataProvider() instanceof FilterRowComboBoxDataProvider
// available items
List<?> allValues = ((FilterRowComboBoxDataProvider) getComboBoxDataProvider()).getAllValues(getColumnIndex());
List<?> visibleValues = getComboBoxDataProvider().getValues(getColumnIndex(), getRowIndex());
List<?> diffValues = new ArrayList<>(allValues);
HashSet<?> diffValues = new HashSet<>(allValues);
diffValues.removeAll(visibleValues);
Copy link

@mmnze mmnze Oct 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realized: This does not actually solve the issue:
The expensive contains operation within the removeAll is not executed on "this" (ie. diffValues) but the method argument, an ArrayList in this case. See HashSet, l. 175 (Java 11).
Sorry, my bad.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before I changed from ArrayList to HashSet I tested locally with this code:

    @Test
    void testListPerformance() {
        ArrayList<String> all = new ArrayList<>();
        ArrayList<String> flanders = new ArrayList<>();

        for (int i = 0; i < 100_000; i++) {
            all.add("Simpson_" + i);
            all.add("Flanders_" + i);
            flanders.add("Flanders_" + i);
        }

        long start = System.currentTimeMillis();

        ArrayList<String> diff = new ArrayList<>(all);
        diff.removeAll(flanders);

        long end = System.currentTimeMillis();

        System.out.println("ArrayList#removeAll(ArrayList) - " + (end - start));

        start = System.currentTimeMillis();

        HashSet<String> diffSet = new HashSet<>(all);
        diffSet.removeAll(flanders);

        end = System.currentTimeMillis();

        System.out.println("HashSet#removeAll(ArrayList) - " + (end - start));

        start = System.currentTimeMillis();

        HashSet<String> diffSetSet = new HashSet<>(all);
        diffSetSet.removeAll(new HashSet<>(flanders));

        end = System.currentTimeMillis();

        System.out.println("HashSet#removeAll(HashSet) - " + (end - start));
    }

If diff is a List, the operation takes about 60s. The diffSet variant takes around 30ms. I would say that the test is similar to the code in discussion. Or what am I missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I found out why my test was working. It strongly depends on the size of the collection passed as parameter. As long as the collection is smaller than the "this" collection, things work fast. If the collection is bigger, the performance is a mess.

I adjusted the collection creation to verify it:

        for (int i = 0; i < 100_000; i++) {
            all.add("Simpson_" + i);
            all.add("Flanders_" + i);
            flanders.add("Flanders_" + i);
            flanders.add("FlandersO_" + i);
        }

        for (int i = 0; i < 10_000; i++) {
            flanders.add("FlandersXxx_" + i);
        }

Now the HashSet#removeAll(ArrayList) takes 123s while HashSet#removeAll(HashSet) is still at about 30ms.


// ensure that items that are not selected don't get added, to avoid
Expand Down
75 changes: 48 additions & 27 deletions ...nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/widget/NatCombo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

import org.eclipse.collections.api.map.primitive.MutableObjectIntMap;
import org.eclipse.collections.api.set.primitive.IntSet;
import org.eclipse.collections.impl.factory.primitive.IntSets;
import org.eclipse.collections.impl.factory.primitive.ObjectIntMaps;
import org.eclipse.jface.viewers.ArrayContentProvider;
import org.eclipse.jface.viewers.LabelProvider;
import org.eclipse.jface.viewers.TableViewer;
Expand All @@ -32,7 +37,6 @@
import org.eclipse.nebula.widgets.nattable.style.IStyle;
import org.eclipse.nebula.widgets.nattable.style.VerticalAlignmentEnum;
import org.eclipse.nebula.widgets.nattable.ui.matcher.LetterOrDigitKeyEventMatcher;
import org.eclipse.nebula.widgets.nattable.util.ArrayUtil;
import org.eclipse.nebula.widgets.nattable.util.GUIHelper;
import org.eclipse.swt.SWT;
import org.eclipse.swt.events.ControlEvent;
Expand Down Expand Up @@ -132,6 +136,15 @@ public class NatCombo extends Composite {
*/
protected java.util.List<String> itemList;

/**
* Mapping of item to index in the {@link #itemList}. Used for performance
* optimizations of index related operations, e.g. getting the index of an
* item.
*
* @since 2.5
*/
private MutableObjectIntMap<String> itemMap;

/**
* Map used to hold the selection state of items in the drop. Needed to
* maintain state when filtering
Expand Down Expand Up @@ -525,6 +538,12 @@ public NatCombo(Composite parent, IStyle cellStyle, int maxVisibleItems, int sty
public void setItems(String[] items) {
if (items != null) {
this.itemList = Arrays.asList(items);

this.itemMap = ObjectIntMaps.mutable.empty();
for (int i = 0; i < this.itemList.size(); i++) {
this.itemMap.put(this.itemList.get(i), i);
}

this.selectionStateMap = new LinkedHashMap<>();
for (String item : items) {
this.selectionStateMap.put(item, Boolean.FALSE);
Expand Down Expand Up @@ -1119,11 +1138,11 @@ public int getSelectionIndex() {
if (this.selectionStateMap != null) {
for (String item : this.selectionStateMap.keySet()) {
if (this.selectionStateMap.get(item)) {
return this.itemList.indexOf(item);
return getItemIndex(item);
}
}
} else if (!this.text.isDisposed()) {
return this.itemList.indexOf(this.text.getText());
return getItemIndex(this.text.getText());
}
return -1;
}
Expand All @@ -1141,22 +1160,15 @@ public int getSelectionIndex() {
*/
public int[] getSelectionIndices() {
if (this.selectionStateMap != null) {
List<Integer> selectedIndices = new ArrayList<>();
for (String item : this.selectionStateMap.keySet()) {
if (this.selectionStateMap.get(item)) {
selectedIndices.add(this.itemList.indexOf(item));
}
}
int[] indices = new int[selectedIndices.size()];
for (int i = 0; i < selectedIndices.size(); i++) {
indices[i] = selectedIndices.get(i);
}
return indices;
return this.selectionStateMap.entrySet().stream()
.filter(entry -> entry.getValue())
.mapToInt(entry -> getItemIndex(entry.getKey()))
.toArray();
} else {
String[] selectedItems = getTextAsArray();
int[] result = new int[selectedItems.length];
for (int i = 0; i < selectedItems.length; i++) {
result[i] = this.itemList.indexOf(selectedItems[i]);
result[i] = getItemIndex(selectedItems[i]);
}
return result;
}
Expand All @@ -1169,18 +1181,26 @@ public int[] getSelectionIndices() {
*/
public int getSelectionCount() {
if (this.selectionStateMap != null) {
List<Integer> selectedIndices = new ArrayList<>();
for (String item : this.selectionStateMap.keySet()) {
if (this.selectionStateMap.get(item)) {
selectedIndices.add(this.itemList.indexOf(item));
}
}
return selectedIndices.size();
return (int) this.selectionStateMap.entrySet().stream()
.filter(entry -> entry.getValue())
.count();
} else {
return getTextAsArray().length;
}
}

/**
* Returns the zero-relative index of the requested item.
*
* @param item
* the item of which the index is requested.
* @return the index of the item.
* @since 2.5
*/
protected int getItemIndex(String item) {
return this.itemMap.get(item);
}

/**
* Returns an array of <code>String</code>s that are currently selected in
* the receiver. The order of the items is unspecified. An empty array
Expand Down Expand Up @@ -1273,9 +1293,9 @@ public void select(int index) {
public void select(int[] indices) {
if (!getDropdownTable().isDisposed()) {
getDropdownTable().select(indices);
List<Integer> indicesList = ArrayUtil.asIntegerList(indices);
fipro78 marked this conversation as resolved.
Show resolved Hide resolved
IntSet indicesSet = IntSets.immutable.of(indices);
for (int i = 0; i < this.itemList.size(); i++) {
this.selectionStateMap.put(this.itemList.get(i), indicesList.contains(i));
this.selectionStateMap.put(this.itemList.get(i), indicesSet.contains(i));
}
this.text.setText(getTransformedTextForSelection());
} else {
Expand Down Expand Up @@ -1494,11 +1514,12 @@ protected String[] getTransformedSelection() {
* The Strings that represent the selected items
*/
protected void setDropdownSelection(String[] selection) {
java.util.List<String> selectionList = Arrays.asList(selection);
Map<String, Boolean> selectionMap = Arrays.stream(selection).collect(Collectors.toMap(e -> e, e -> Boolean.TRUE));
java.util.List<TableItem> selectedItems = new ArrayList<>();
boolean containsSelectAll = selectionMap.getOrDefault(EditConstants.SELECT_ALL_ITEMS_VALUE, Boolean.FALSE);
for (TableItem item : getDropdownTable().getItems()) {
if (selectionList.contains(EditConstants.SELECT_ALL_ITEMS_VALUE)
|| selectionList.contains(item.getText())) {
if (containsSelectAll
|| selectionMap.getOrDefault(item.getText(), Boolean.FALSE)) {
this.selectionStateMap.put(item.getText(), Boolean.TRUE);
if (this.useCheckbox) {
item.setChecked(true);
Expand Down
14 changes: 6 additions & 8 deletions ...a/widgets/nattable/extension/glazedlists/filterrow/ComboBoxGlazedListsFilterStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2013, 2023 Dirk Fauth and others.
* Copyright (c) 2013, 2024 Dirk Fauth and others.
*
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
Expand All @@ -23,6 +23,7 @@
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.StringJoiner;
import java.util.regex.Pattern;

import org.eclipse.nebula.widgets.nattable.config.IConfigRegistry;
Expand Down Expand Up @@ -191,25 +192,22 @@ protected String getStringFromColumnObject(final int columnIndex, final Object o
FILTER_ROW_COLUMN_LABEL_PREFIX + columnIndex);

if (object instanceof Collection) {
String result = ""; //$NON-NLS-1$
Collection valueCollection = (Collection) object;
StringJoiner joiner = new StringJoiner("|", "(", ")"); //$NON-NLS-1$//$NON-NLS-2$//$NON-NLS-3$
for (Object value : valueCollection) {
if (result.length() > 0) {
result += "|"; //$NON-NLS-1$
}
String convertedValue = displayConverter.canonicalToDisplayValue(
new LayerCell(null, columnIndex, 0),
this.configRegistry,
value).toString();
if (convertedValue.isEmpty()) {
// for an empty String add the regular expression for empty
// String
result += "^$"; //$NON-NLS-1$
joiner.add("^$"); //$NON-NLS-1$
} else {
result += Pattern.quote(convertedValue);
joiner.add(Pattern.quote(convertedValue));
}
}
return "(" + result + ")"; //$NON-NLS-1$//$NON-NLS-2$
return joiner.toString();
}

if (displayConverter != null) {
Expand Down