Repository navigation
perf(grid): compile filter predicates once instead of per row (#1550) - #1611
Merged
Merged
Conversation
_PredicateNode.evaluate re-derived everything from the operator and operand strings for every row: op.toUpperCase().trim(), targetValue.toLowerCase() and double.tryParse(targetValue), the lower-cased and trimmed IN list (O(rows x list)) and the BETWEEN bounds. The constructor now computes the operator, the lower-cased target, its number, the normalised IN set and the BETWEEN bounds once; evaluate only reads the cell. Semantics are unchanged: numeric comparison when both sides parse, otherwise case-insensitive string comparison, NULL / null / empty are null. Closes #1550
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
_PredicateNode.evaluatere-derived everything from the raw strings for every row:op.toUpperCase().trim(),targetValue.toLowerCase()anddouble.tryParse(targetValue)(the target never changes), the lower-cased and trimmedINlist on each row (O(rows x list)), and theBETWEENbounds.The constructor now resolves them once:
_upperOp,_lowerTarget,_numTarget,_inSet(aSet<String>of normalised values) and_numMin/_numMax.evaluateis aswitchon the resolved operator and only reads the cell. Semantics are as before: numeric comparison when both sides parse, otherwise case-insensitive string comparison;NULL/null/ empty count as null;BETWEENwithout both bounds falls through to the comparison path.The only per-row string work left is on the cell side (
toLowerCase,trim,tryParseof the cell), which cannot be hoisted.This touches the node class only; the lexer changes in #1607 are in the same file but different lines.
Tests (not run locally; CI will)
test/features/workspace/grid_filter_predicate_test.dart:IN/NOT INwith mixed case and spaces, numeric and stringBETWEEN, numeric vs string comparison (20.0=20,price = abc),:and!=ignoring case, andIS NULLforNULL/null/ empty.The before / after timing on 50k rows asked for in the issue is not included; the harness is not in the repo.
Closes #1550