Repository navigation
perf(grid): regex-free formatNum and comma-only copies in selection stats (#1531) - #1614
Merged
Merged
Conversation
GridSelectionCalcEngine.formatNum compiled two RegExp objects and ran two
replace passes for every stat on every selection change, and compute made a
replaceAll(',', '') copy for every selected cell.
formatNum walks the code units from the end to drop trailing zeros and a
dangling dot. compute strips commas only from cells that contain one. Output
is unchanged.
Closes #1531
Refs #1520
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
formatNumcompiledRegExp(r'0+$')andRegExp(r'\.$')and ran two replace passes for each of Sum / Avg / Median / Min / Max / StdDev on every selection change. It now walks the code units from the end: drop trailing0s, then one., and slice once only if something was removed. Integer-valued numbers still go through thetoInt()branch.computemadetrimmed.replaceAll(',', '')for every selected cell. It now makes the copy only for cells that contain a comma; the value that goes to the distinct set and totryParseis unchanged.Not done: the custom allocation-free number parser the #1520 issue proposes.
double.tryParseis correctly rounded and a hand-written parser has to match it digit for digit, which is a bigger risk than the copy it saves, so that issue stays open (Refs).Tests (not run locally; CI will)
grid_selection_calc_format_test.dart:formatNumon whole, fractional, repeating and round-to-integer values, andcomputeon a mix of1,000, padded2,500.5,NULL, empty, text and duplicate values (counts, distinct, sum, min, max, median).Closes #1531
Refs #1520