Repository navigation
perf(db): reuse one Intl.Collator per locale compare options #2438
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.
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
Open
AI091
wants to merge
1
commit into
TanStack:main
Choose a base branch
from
AI091:perf/db-reuse-locale-collator
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+388
−3
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@tanstack/db': patch | ||
| --- | ||
|
|
||
| Compare strings under locale collation with one `Intl.Collator` per compare-options object instead of calling `localeCompare` for each comparison. The order is unchanged. Building a collator on every comparison was slow on Hermes for Android, where each one is a Java object. |
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| # Cached locale collator oracle review | ||
|
|
||
| Reviewed source head: the commit that adds this record on | ||
| `perf/db-reuse-locale-collator`. It changes `ascComparator` in | ||
| `packages/db/src/utils/comparison.ts` and adds the | ||
| `comparison.locale-option-history` property to | ||
| `packages/db/tests/comparison-oracle.property.test.ts`. | ||
|
|
||
| ## Contract and evidence | ||
|
|
||
| Under locale string collation, `ascComparator` orders two strings as | ||
| `Intl.Collator(locale, localeOptions)` does. The authority is | ||
| `StringCollationConfig` and `CompareOptions`, and ECMA-402, which defines | ||
| `String.prototype.localeCompare(that, locales, options)` as a comparison by a new | ||
| `Intl.Collator(locales, options)`. | ||
|
|
||
| The change keeps this result and changes its cost. Before, every comparison | ||
| called `localeCompare`, and Hermes for Android builds a Java collator for each | ||
| call. Now each compare-options object keeps the collator it first builds, in a | ||
| `WeakMap` keyed by that object. | ||
|
|
||
| The cache adds state across comparisons, which the stateless comparison laws | ||
| cannot see. The new property judges histories of comparisons under mixed option | ||
| sets. Its model builds a new `Intl.Collator` for every step and imports no | ||
| production helper. | ||
|
|
||
| ## Mutant results | ||
|
|
||
| The production source was restored after each run. Command: | ||
| `pnpm --filter @tanstack/db exec vitest run --coverage.enabled=false tests/comparison-oracle.property.test.ts -t locale-option-history`. | ||
|
|
||
| | Production | Outcome | | ||
| | --- | --- | | ||
| | Parent (`localeCompare` per call) | 11 of 11 passed. The law is unchanged, so the parent passes it. | | ||
| | This change | 11 of 11 passed. | | ||
| | One collator kept for every option set | 7 failures. Shrunk to one step: `item2` against `item10` under numeric `en-US` after plain `en-US`. | | ||
| | One collator kept per locale, ignoring `localeOptions` | 7 failures, same shrunk step. | | ||
|
|
||
| ## ORC-012 requirement audit | ||
|
|
||
| | Requirement | Outcome | | ||
| | --- | --- | | ||
| | ORC-001 | Applicable. The law and authority are above and in the property. Limits: ascending comparisons through `ascComparator`; option objects are not changed after use; Collection and query paths keep their owners in the coverage map. | | ||
| | ORC-002 | Applicable. The model calls `new Intl.Collator(locale, localeOptions).compare` for each step and imports nothing from production. | | ||
| | ORC-003 | Applicable. The property's law, authority and limits state the contract; the grammar, driver and check are separate functions beside it. | | ||
| | ORC-004 | Applicable. Histories have 1 to 12 steps over seven option sets (default locale; `en-US` plain, `numeric`, `sensitivity: "base"`, `caseFirst: "upper"`; `sv`; `de`) and eight strings. Every two sets that share a locale order some pair differently. Each step gets the set's shared options object or a fresh copy. | | ||
| | ORC-005 | Applicable. The driver calls the exported `ascComparator` with `CompareOptions` built as a Collection builds them, and records the sign of each result. | | ||
| | ORC-006 | Applicable. Three calibrations: one collator for every option set, one collator per locale, and a construction whose reach observation is removed. Each is rejected under its named law. The production mutants above confirm the first two. | | ||
| | ORC-007 | Applicable. A fixed-seed campaign (`20260911`) and a random campaign share one budget. The property is registered as `comparison.locale-option-history`, so the replay variables select it. | | ||
| | ORC-008 | Inapplicable. The model is a stateless function of each step. | | ||
| | ORC-009 | Applicable. "Option set" names one `CompareOptions` configuration; "locale string collation" is `stringSort: 'locale'`. | | ||
| | ORC-010 | Applicable. A violation names the step, its option set, and the owed and observed signs. The driver has no cleanup. | | ||
| | ORC-011 | Not provided. No second formulation judges the cache; the stated limit is the bounded grammar above. | | ||
| | ORC-013 | Inapplicable. No threshold or range law. | | ||
| | ORC-014 | Applicable, limited. The host engine's `Intl` supplies the premise. The oracle runs on Node's ICU, not on Hermes. On Hermes for Android, a 30,000-string sort gave the same order with a cached collator as with `localeCompare`. | | ||
| | ORC-015 | Applicable. Declared states: "an earlier option set orders the pair differently" and "an earlier option set with the same locale orders the pair differently". The constructed history keeps at most 10 generated steps between its two added steps, so it stays inside the 12-step grammar. Every built history records its required states; the check fails when the driver's reach observations miss one, and the third calibration shows that failure. Generated reach in one run: 77 and 64 of 100 histories (fixed seed), 73 and 59 of 100 (random). | | ||
| | ORC-016 | Applicable. The file registers the property only through `oracleProperty` and adds no unit tests. | | ||
|
|
||
| ## Open for maintainers | ||
|
|
||
| - **Option objects changed after use.** The old code read a changed | ||
| `localeOptions` or `locale` on the next comparison; the cache keeps the | ||
| collator it built. An index built under the old options keeps its stored | ||
| order, so a change after use already left indexes inconsistent. The law states this as a | ||
| limit and needs a maintainer decision. | ||
| - **Runtimes without `Intl.Collator`.** `localeCompare` works without `Intl` in | ||
| some engine builds (for example Node built without Intl support). The default | ||
| comparison now needs `Intl.Collator`. Explicit locales already need `Intl` | ||
| through `Intl.getCanonicalLocales` in index matching. | ||
| - **RED.** A performance change under an unchanged law passes on the parent. | ||
| The `inArray` constant-list review (`perf-in-constant-set.md`) records the same | ||
| shape: mutants and a requirement audit, no parent failure. | ||
|
|
||
| ## Performance evidence | ||
|
|
||
| Hermes, Android emulator (x86_64, 4 cores), sorting 30,000 random strings | ||
| (about 489,000 comparisons), three runs each: `localeCompare` 11.0–12.3 µs per | ||
| comparison, a cached `Intl.Collator` 1.16–1.21 µs, a plain comparison | ||
| 0.19–0.22 µs. The two collation methods produced the same order. |
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirm that supported runtimes provide
Intl, or keep a fallback.The default locale string comparison now calls
new Intl.Collator(...)atpackages/db/src/utils/comparison.ts:33. The previous code calleda.localeCompare(b, opts.locale, opts.localeOptions)and did not referenceIntl. In a runtime that provideslocaleComparebut notIntl, the new path can throw aReferenceErrorinstead of returning an order.The package does not declare an
enginesfield, and the inspected docs do not state anIntlrequirement. The review record lists this as an open maintainer decision (docs/contributing/oracle-reviews/perf-locale-collator.md:66-69). Either:Intlis a runtime requirement for the default string comparison, orlocaleComparefallback whenIntlis unavailable.Record the chosen contract in the review record.