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

Firestore: Choose DEFAULT_RELATIVE_INDEX_READ_COST_PER_DOCUMENT value based on the browser, rather than hardcoding 8 #7929

Merged
merged 2 commits into from
Jan 8, 2024
Merged

Firestore: Choose DEFAULT_RELATIVE_INDEX_READ_COST_PER_DOCUMENT value based on the browser, rather than hardcoding 8 #7929

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
b989400
Firestore: query_engine.ts: choose DEFAULT_RELATIVE_INDEX_READ_COST_P…
dconeybe Jan 8, 2024
b5424e3
yarn changeset
dconeybe Jan 8, 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
6 changes: 6 additions & 0 deletions .changeset/witty-comics-rule.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'firebase': patch
'@firebase/firestore': patch
---

Tweak the automatic index creation parameters to use more optimal values for the platform/browser detected at runtime.
20 changes: 16 additions & 4 deletions packages/firestore/src/local/query_engine.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
* limitations under the License.
*/

import { getUA, isSafari } from '@firebase/util';

import {
LimitType,
newQueryComparator,
Expand Down Expand Up @@ -47,6 +49,7 @@ import { LocalDocumentsView } from './local_documents_view';
import { PersistencePromise } from './persistence_promise';
import { PersistenceTransaction } from './persistence_transaction';
import { QueryContext } from './query_context';
import { SimpleDb } from './simple_db';

const DEFAULT_INDEX_AUTO_CREATION_MIN_COLLECTION_SIZE = 100;

Expand All @@ -55,10 +58,19 @@ const DEFAULT_INDEX_AUTO_CREATION_MIN_COLLECTION_SIZE = 100;
* (([index, docKey] + [docKey, docContent]) per document in the result set)
* / ([docKey, docContent] per documents in full collection scan) coming from
* experiment [enter PR experiment URL here].
* TODO(b/299284287) Choose a value appropriate for the browser/OS combination,
* as determined by more data points from running the experiment.
*/
const DEFAULT_RELATIVE_INDEX_READ_COST_PER_DOCUMENT = 8;
function getDefaultRelativeIndexReadCostPerDocument(): number {
// These values were derived from an experiment where several members of the
// Firestore SDK team ran a performance test in various environments.
// Googlers can see b/299284287 for details.
if (isSafari()) {
return 8;
} else if (SimpleDb.getAndroidVersion(getUA()) > 0) {
return 6;
} else {
return 4;
}
}

/**
* The Firestore query engine.
Expand Down Expand Up @@ -113,7 +125,7 @@ export class QueryEngine {
DEFAULT_INDEX_AUTO_CREATION_MIN_COLLECTION_SIZE;

relativeIndexReadCostPerDocument =
DEFAULT_RELATIVE_INDEX_READ_COST_PER_DOCUMENT;
getDefaultRelativeIndexReadCostPerDocument();

/** Sets the document view to query against. */
initialize(
Expand Down
Loading