Skip to content

Commit 5ea8849

Browse files
hokolomopohokolomopo
committed
[IMP] filter: fix horrible performances with huge data filters
On a spreadsheet with 10,000 rows, opening the filter menu and filtering all the values is really slow. This commit improves the performance of the existing functions, and adds a `load more` button in the filter menu to avoid creating 10000 owl components and DOM elements. On a data filter with 10,000 rows: - `getFilterHiddenValues` (when opening filter menu): ~1000ms => ~70ms - `updateHiddenRows` (when filtering all values): ~543ms => ~8ms Task: 4658998
1 parent 8d181ea commit 5ea8849

File tree

5 files changed

+110
-34
lines changed

5 files changed

+110
-34
lines changed

‎src/components/filters/filter_menu_value_list/filter_menu_value_list.ts‎

Lines changed: 58 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,11 @@ interface Value {
1717

1818
interface State {
1919
values: Value[];
20+
displayedValues: Value[];
2021
textFilter: string;
2122
selectedValue: string | undefined;
23+
numberOfDisplayedValues: number;
24+
hasMoreValues: boolean;
2225
}
2326

2427
export class FilterMenuValueList extends Component<Props, SpreadsheetChildEnv> {
@@ -31,8 +34,11 @@ export class FilterMenuValueList extends Component<Props, SpreadsheetChildEnv> {
3134

3235
private state: State = useState({
3336
values: [],
37+
displayedValues: [],
3438
textFilter: "",
3539
selectedValue: undefined,
40+
numberOfDisplayedValues: 50,
41+
hasMoreValues: false,
3642
});
3743

3844
private searchBar = useRef("filterMenuSearchBar");
@@ -41,10 +47,12 @@ export class FilterMenuValueList extends Component<Props, SpreadsheetChildEnv> {
4147
onWillUpdateProps((nextProps: Props) => {
4248
if (!deepEquals(nextProps.filterPosition, this.props.filterPosition)) {
4349
this.state.values = this.getFilterHiddenValues(nextProps.filterPosition);
50+
this.computeDisplayedValues();
4451
}
4552
});
4653

4754
this.state.values = this.getFilterHiddenValues(this.props.filterPosition);
55+
this.computeDisplayedValues();
4856
}
4957

5058
private getFilterHiddenValues(position: Position): Value[] {
@@ -67,27 +75,33 @@ export class FilterMenuValueList extends Component<Props, SpreadsheetChildEnv> {
6775

6876
const cellValues = cells.map((val) => val.cellValue);
6977
const filterValues = filterValue?.filterType === "values" ? filterValue.hiddenValues : [];
70-
71-
const strValues = [...cellValues, ...filterValues];
72-
const normalizedFilteredValues = filterValues.map(toLowerCase);
73-
74-
// Set with lowercase values to avoid duplicates
75-
const normalizedValues = [...new Set(strValues.map(toLowerCase))];
76-
77-
const sortedValues = normalizedValues.sort((val1, val2) =>
78-
val1.localeCompare(val2, undefined, { numeric: true, sensitivity: "base" })
79-
);
80-
81-
return sortedValues.map((normalizedValue) => {
82-
let checked = false;
83-
if (filterValue?.filterType !== "criterion") {
84-
checked = normalizedFilteredValues.findIndex((val) => val === normalizedValue) === -1;
78+
const normalizedFilteredValues = new Set(filterValues.map(toLowerCase));
79+
80+
const set = new Set<string>();
81+
const values: (Value & { normalizedValue: string })[] = [];
82+
const addValue = (value: string) => {
83+
const normalizedValue = toLowerCase(value);
84+
if (!set.has(normalizedValue)) {
85+
values.push({
86+
string: value || "",
87+
checked:
88+
filterValue?.filterType !== "criterion"
89+
? !normalizedFilteredValues.has(normalizedValue)
90+
: false,
91+
normalizedValue,
92+
});
93+
set.add(normalizedValue);
8594
}
86-
return {
87-
checked,
88-
string: strValues.find((val) => toLowerCase(val) === normalizedValue) || "",
89-
};
90-
});
95+
};
96+
cellValues.forEach(addValue);
97+
filterValues.forEach(addValue);
98+
99+
return values.sort((val1, val2) =>
100+
val1.normalizedValue.localeCompare(val2.normalizedValue, undefined, {
101+
numeric: true,
102+
sensitivity: "base",
103+
})
104+
);
91105
}
92106

93107
checkValue(value: Value) {
@@ -102,29 +116,43 @@ export class FilterMenuValueList extends Component<Props, SpreadsheetChildEnv> {
102116
}
103117

104118
selectAll() {
105-
this.displayedValues.forEach((value) => (value.checked = true));
106-
this.updateHiddenValues();
119+
this.state.displayedValues.forEach((value) => (value.checked = true));
120+
this.props.onUpdateHiddenValues([]);
107121
}
108122

109123
clearAll() {
110-
this.displayedValues.forEach((value) => (value.checked = false));
111-
this.updateHiddenValues();
124+
this.state.displayedValues.forEach((value) => (value.checked = false));
125+
const hiddenValues = this.state.values.map((val) => val.string);
126+
this.props.onUpdateHiddenValues(hiddenValues);
112127
}
113128

114129
updateHiddenValues() {
115130
const hiddenValues = this.state.values.filter((val) => !val.checked).map((val) => val.string);
116131
this.props.onUpdateHiddenValues(hiddenValues);
117132
}
118133

119-
get displayedValues() {
120-
if (!this.state.textFilter) {
121-
return this.state.values;
122-
}
123-
return fuzzyLookup(this.state.textFilter, this.state.values, (val) => val.string);
134+
updateSearch(ev: Event) {
135+
const target = ev.target as HTMLInputElement;
136+
this.state.textFilter = target.value;
137+
this.state.selectedValue = undefined;
138+
this.computeDisplayedValues();
139+
}
140+
141+
computeDisplayedValues() {
142+
const values = !this.state.textFilter
143+
? this.state.values
144+
: fuzzyLookup(this.state.textFilter, this.state.values, (val) => val.string);
145+
this.state.displayedValues = values.slice(0, this.state.numberOfDisplayedValues);
146+
this.state.hasMoreValues = values.length > this.state.numberOfDisplayedValues;
147+
}
148+
149+
loadMoreValues() {
150+
this.state.numberOfDisplayedValues += 100;
151+
this.computeDisplayedValues();
124152
}
125153

126154
onKeyDown(ev: KeyboardEvent) {
127-
const displayedValues = this.displayedValues;
155+
const displayedValues = this.state.displayedValues;
128156

129157
if (displayedValues.length === 0) return;
130158

‎src/components/filters/filter_menu_value_list/filter_menu_value_list.xml‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
class="w-100 o-input my-2"
1010
t-ref="filterMenuSearchBar"
1111
type="text"
12-
t-model="state.textFilter"
12+
t-on-input="updateSearch"
1313
placeholder="Search..."
1414
t-on-keydown="onKeyDown"
1515
/>
@@ -22,7 +22,7 @@
2222
t-ref="filterValueList"
2323
t-on-click="this.clearScrolledToValue"
2424
t-on-scroll="this.clearScrolledToValue">
25-
<t t-foreach="displayedValues" t-as="value" t-key="value.string">
25+
<t t-foreach="state.displayedValues" t-as="value" t-key="value.string">
2626
<FilterMenuValueItem
2727
onClick="() => this.checkValue(value)"
2828
onMouseMove="() => this.onMouseMove(value)"
@@ -33,7 +33,13 @@
3333
/>
3434
</t>
3535
<div
36-
t-if="displayedValues.length === 0"
36+
t-if="state.hasMoreValues"
37+
class="o-filter-load-more o-button-link d-flex justify-content-center py-1"
38+
t-on-click="this.loadMoreValues">
39+
Load more...
40+
</div>
41+
<div
42+
t-if="state.displayedValues.length === 0"
3743
class="o-filter-menu-no-values d-flex align-items-center justify-content-center w-100 h-100 ">
3844
No results
3945
</div>

‎src/plugins/ui_stateful/filter_evaluation.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,9 +181,10 @@ export class FilterEvaluationPlugin extends UIPlugin {
181181
if (filterValue.filterType === "values") {
182182
const filteredValues = filterValue.hiddenValues?.map(toLowerCase);
183183
if (!filteredValues) continue;
184+
const filteredValuesSet = new Set(filteredValues);
184185
for (let row = filteredZone.top; row <= filteredZone.bottom; row++) {
185186
const value = this.getCellValueAsString(sheetId, filter.col, row);
186-
if (filteredValues.includes(value)) {
187+
if (filteredValuesSet.has(value)) {
187188
hiddenRows.add(row);
188189
}
189190
}

‎tests/table/__snapshots__/filter_menu_component.test.ts.snap‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,7 @@ exports[`Filter menu component Filter Tests Filter menu is correctly rendered 1`
210210
211211
212212
213+
213214
</div>
214215
</div>
215216
</div>

‎tests/table/filter_menu_component.test.ts‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,46 @@ describe("Filter menu component", () => {
393393
).not.toContain("Sort ascending (A ⟶ Z)");
394394
});
395395

396+
test("Only the first few values are displayed by default", async () => {
397+
for (let i = 1; i < 61; i++) {
398+
setCellContent(model, `A${i}`, `${i}`);
399+
}
400+
createTableWithFilter(model, "A1:A61");
401+
await nextTick();
402+
await openFilterMenu();
403+
404+
expect(fixture.querySelectorAll(".o-filter-menu-value")).toHaveLength(50);
405+
406+
await simulateClick(".o-filter-load-more");
407+
expect(fixture.querySelectorAll(".o-filter-menu-value")).toHaveLength(60);
408+
});
409+
410+
test("Search/Clear/Select all all works when some values are not displayed", async () => {
411+
for (let i = 1; i < 61; i++) {
412+
setCellContent(model, `A${i}`, `${i}`);
413+
}
414+
createTableWithFilter(model, "A1:A61");
415+
await nextTick();
416+
await openFilterMenu();
417+
expect(fixture.querySelectorAll(".o-filter-menu-value")).toHaveLength(50); // Only 50 values are displayed
418+
419+
await simulateClick(".o-filter-menu-actions .o-button-link:nth-of-type(2)");
420+
expect(getFilterMenuValues().every((val) => !val.isChecked)).toBe(true);
421+
await simulateClick(".o-filter-menu-confirm");
422+
expect(model.getters.getFilterHiddenValues({ sheetId, col: 0, row: 0 })).toHaveLength(60);
423+
424+
await openFilterMenu();
425+
expect(getFilterMenuValues().every((val) => !val.isChecked)).toBe(true);
426+
await simulateClick(".o-filter-menu-actions .o-button-link:nth-of-type(1)");
427+
expect(getFilterMenuValues().every((val) => val.isChecked)).toBe(true);
428+
await simulateClick(".o-filter-menu-confirm");
429+
expect(model.getters.getFilterHiddenValues({ sheetId, col: 0, row: 0 })).toHaveLength(0);
430+
431+
await openFilterMenu();
432+
await setInputValueAndTrigger(".o-filter-menu input", "59");
433+
expect(getFilterMenuValues().map((val) => val.value)).toEqual(["59"]);
434+
});
435+
396436
describe("Filter criterion tests", () => {
397437
beforeEach(async () => {
398438
createTableWithFilter(model, "A1:A5");

0 commit comments

Comments
 (0)