Skip to content

Port performance optimizations to speed up reading large collections from Android #1433

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

Merged
merged 30 commits into from
Dec 22, 2018
Merged

Port performance optimizations to speed up reading large collections from Android #1433

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
3da020e
1
var-const Dec 14, 2018
9717fef
Compiles
var-const Dec 14, 2018
204443e
Compiles, pt 2
var-const Dec 14, 2018
c56820b
Compiles, pt 3m
var-const Dec 14, 2018
f3174ca
[AUTOMATED]: Prettier Code Styling
var-const Dec 14, 2018
36c53d3
Some fixes
var-const Dec 14, 2018
942b0fa
Very hacky version works
var-const Dec 15, 2018
7767b60
Most unit tests pass
var-const Dec 17, 2018
a1ad6d2
[AUTOMATED]: Prettier Code Styling
var-const Dec 17, 2018
cc29731
Undo temp/accidental changes
var-const Dec 17, 2018
307e0b3
applyRemoteEvent, sized entries
var-const Dec 18, 2018
62c627b
Fix failing tests
var-const Dec 18, 2018
8f293eb
Serializer
var-const Dec 18, 2018
4e23f3b
[AUTOMATED]: Prettier Code Styling
var-const Dec 18, 2018
fb751dd
small cleanup
var-const Dec 18, 2018
431f618
Fix accidental
var-const Dec 18, 2018
1681f3d
Comment
var-const Dec 18, 2018
55c7ff3
Review feedback 1, test
var-const Dec 19, 2018
5afa305
[AUTOMATED]: Prettier Code Styling
var-const Dec 19, 2018
56eefbc
Review feedback 1
var-const Dec 20, 2018
5e506fa
Review feedback 2
var-const Dec 20, 2018
7b11cec
[AUTOMATED]: Prettier Code Styling
var-const Dec 20, 2018
1a32b22
Review feedback 3
var-const Dec 21, 2018
17a69a6
Review feedback
var-const Dec 22, 2018
4c270c7
Appease linter
var-const Dec 22, 2018
512667e
Fix node tests
var-const Dec 22, 2018
6cb4bfb
Appease linter 2
var-const Dec 22, 2018
77bf92e
[AUTOMATED]: Prettier Code Styling
var-const Dec 22, 2018
e7b8c8e
Comment
var-const Dec 22, 2018
a0d25f5
Merge branch 'master' into varconst/port-android-1000-reads-3
var-const Dec 22, 2018
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
78 changes: 78 additions & 0 deletions packages/firestore/src/local/indexeddb_remote_document_cache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,19 @@

import { Query } from '../core/query';
import {
DocumentKeySet,
documentKeySet,
DocumentMap,
documentMap,
DocumentSizeEntry,
DocumentSizeEntries,
nullableMaybeDocumentMap,
NullableMaybeDocumentMap,
MaybeDocumentMap,
maybeDocumentMap
} from '../model/collections';
import { Document, MaybeDocument, NoDocument } from '../model/document';
import { SortedMap } from '../util/sorted_map';
import { DocumentKey } from '../model/document_key';

import { SnapshotVersion } from '../core/snapshot_version';
Expand Down Expand Up @@ -178,6 +183,72 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache {
});
}

getEntries(
transaction: PersistenceTransaction,
documentKeys: DocumentKeySet
): PersistencePromise<NullableMaybeDocumentMap> {
return this.getSizedEntries(transaction, documentKeys).next(
result => result.maybeDocuments
);
}

/**
* Looks up several entries in the cache.
*
* @param documentKeys The set of keys entries to look up.
* @return A map of MaybeDocuments indexed by key (if a document cannot be
* found, the key will be mapped to null) and a map of sizes indexed by
* key (zero if the key cannot be found).
*/
getSizedEntries(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another approach would be for getSizedEntries to return a SortedMap<DocumentKey, DocumentSizeEntry>. I decided in favor of returning two maps because it makes it easier to avoid code duplication between getEntries and getSizedEntries.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider taking as an argument a function that processes each key into the type of the result, for example fn: (key: DocumentKey, doc: DbRemoteDoc | null) => T. Then, your return type can be PersistencePromise<SortedMap<DocumentKey, T>>. You can avoid code duplication and avoid doing extra work for sizes that way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it out, but I'm not sure I prefer it. The problem is that getEntries in RemoteDocumentChangeBuffer won't be able to return a map of documents directly (due to type difference) and instead would have to build a new map. If extra work for calculating/storing sizes is a concern, it's easy (though ugly) to solve with a flag (or perhaps, more similar to this approach, by having a fn that either updates the sizeMap or is a no-op).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like that makes sense, but since it deals with DbRemoteDoc, I'd keep it internal and have getEntries() and getSizedEntries() functions that wrap it.

I think what Greg is recommending is basically a mapDbEntries() function, but it might be a little simpler to instead have it be a forEachDbEntry(transaction, documentKeys, callback) function that iterates the matching documents and just calls the callback with each raw dbRemoteDocument. That may mean a little bit of redundant code for getEntries() and getSizedEntries() to build up their respective maps, but it seems simpler to me (and perhaps more generically useful, if we had a case where we don't necessarily want to build up a map).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, please take a look.

transaction: PersistenceTransaction,
documentKeys: DocumentKeySet
): PersistencePromise<DocumentSizeEntries> {
let results = nullableMaybeDocumentMap();
let sizeMap = new SortedMap<DocumentKey, number>(DocumentKey.comparator);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, now that getEntries() is no longer a wrapper around getSizedEntries(), can we drop sizeMap and have this be new SortedMap<DocumentKey, DocumentSizeEntry|null> ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First, I don't feel strongly about this. The reason I set it up that way is so that getEntries in RemoteDocumentChangeBuffer can return the MaybeDocumentMap directly. If this were a sorted map of DocumentSizeEntrys, then getEntries would have to create a new MaybeDocumentMap and fill it with just MaybeDocuments.

It's probably not a big deal, so if you think code clarity is more important here, I'll do the change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, sorry! I was confused. I thought it was just so the old getEntries() implementation could return the MaybeDocumentMap directly. But I see now that RemoteDocumentChangeBuffer.getEntries() ends up calling getSizedEntries() and using the sizes and also passing the MaybeDocumentMap straight through. So it needs both, and the way it's structured right now makes sense.

So nevermind. Please keep it the way it is.

if (documentKeys.isEmpty()) {
return PersistencePromise.resolve({ maybeDocuments: results, sizeMap });
}

const range = IDBKeyRange.bound(
documentKeys.first()!.path.toArray(),
documentKeys.last()!.path.toArray()
);
let key = documentKeys.first();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is built upon the idea that control.skip() makes the algorithm more efficient. Will be happy with any feedback.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me. I might name it nextKey though?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


return remoteDocumentsStore(transaction)
.iterate({ range }, (potentialKeyRaw, dbRemoteDoc, control) => {
const potentialKey = DocumentKey.fromSegments(potentialKeyRaw);
while (DocumentKey.comparator(key!, potentialKey) != 1) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would use < 1 instead. Even though this comparison function might return {-1, 0, 1}, some comparison functions return { < 0, 0, > 0}, which will break here for values > 1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thanks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to this except I'd use <= 0 😁 (technically the comparator could return 0.5 or something).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

if (key!.isEqual(potentialKey)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe not a big deal, but you could store the comparator result and then use === 0 here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm concerned it would complicate the loop somewhat.

results = results.insert(
key!,
this.serializer.fromDbRemoteDocument(dbRemoteDoc)
);
sizeMap = sizeMap.insert(key!, dbDocumentSize(dbRemoteDoc));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whether to update the sizeMap could be controlled by a flag to avoid doing unnecessary work when this function is invoked via getEntries, but it might be overkill.

} else {
results = results.insert(key!, null);
sizeMap = sizeMap.insert(key!, 0);
}

key = documentKeys.firstAfter(key!);
if (!key) {
control.done();
return;
}
control.skip(key!.path.toArray());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should go outside the while loop?

Alternatively, while trying to figure out this code I ended up tweaking it a bit. If you think it's clearer, you can adopt it:

        while (DocumentKey.comparator(key!, potentialKey) < 0) {
          // Key not found in cache.
          results = results.insert(key!, null);
          sizeMap = sizeMap.insert(key!, 0);
          key = documentKeys.firstAfter(key!);
        }

        if (key && key!.isEqual(potentialKey)) {
          // Key found in cache.
          results = results.insert(
            key!,
            this.serializer.fromDbRemoteDocument(dbRemoteDoc)
          );
          sizeMap = sizeMap.insert(key!, dbDocumentSize(dbRemoteDoc));
          key = documentKeys.firstAfter(key!);
        }

        // Skip to the next key (if there is one).
        if (key) {
          control.skip(key!.path.toArray());
        } else {
          control.done();
        }

Not a big deal either way though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adopted (with very slight modification). Thanks, I didn't clean up this loop too much because I thought it might change a lot during the review.

}
})
.next(() => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a comment.

// The rest of the keys must not be in the cache.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done (with minor addition).

while (key) {
results = results.insert(key, null);
sizeMap = sizeMap.insert(key, 0);
key = documentKeys.firstAfter(key!);
}
return { maybeDocuments: results, sizeMap };
});
}

getDocumentsMatchingQuery(
transaction: PersistenceTransaction,
query: Query
Expand Down Expand Up @@ -381,6 +452,13 @@ class IndexedDbRemoteDocumentChangeBuffer extends RemoteDocumentChangeBuffer {
): PersistencePromise<DocumentSizeEntry | null> {
return this.documentCache.getSizedEntry(transaction, documentKey);
}

protected getAllFromCache(
transaction: PersistenceTransaction,
documentKeys: DocumentKeySet
): PersistencePromise<DocumentSizeEntries> {
return this.documentCache.getSizedEntries(transaction, documentKeys);
}
}

export function isDocumentChangeMissingError(err: FirestoreError): boolean {
Expand Down
74 changes: 54 additions & 20 deletions packages/firestore/src/local/local_documents_view.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,14 @@
import { Query } from '../core/query';
import { SnapshotVersion } from '../core/snapshot_version';
import {
documentKeySet,
DocumentKeySet,
DocumentMap,
documentMap,
MaybeDocumentMap,
maybeDocumentMap
maybeDocumentMap,
NullableMaybeDocumentMap,
nullableMaybeDocumentMap
} from '../model/collections';
import { Document, MaybeDocument, NoDocument } from '../model/document';
import { DocumentKey } from '../model/document_key';
Expand Down Expand Up @@ -74,6 +77,25 @@ export class LocalDocumentsView {
});
}

// Returns the view of the given `docs` as they would appear after applying
// all mutations in the given `batches`.
private applyLocalMutationsToDocuments(
transaction: PersistenceTransaction,
docs: NullableMaybeDocumentMap,
batches: MutationBatch[]
): PersistencePromise<NullableMaybeDocumentMap> {
let results = nullableMaybeDocumentMap();
return new PersistencePromise<NullableMaybeDocumentMap>(resolve => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you have to do this in a closure. Consider:

let results = nullableMaybeDocumentMap():
docs.forEach((key, localView) => {
  for (const batch of batches) {
     localView = batch.applyToLocalView(key, localView);
  }
  results = results.insert(key, localView);
});
return PersistencePromise.resolve(results);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I should have mentioned this -- this function doesn't have to be async at all. I wrapped the computation in a promise because I presumed it's time-consuming enough to warrant this. Do you think it should just return results directly?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going further, I don't think this function needs to take a transaction or return a PersistencePromise. It can just be synchronous.

Note that in some cases we do have functions that are "needlessly" asynchronous and return a PersistencePromise when they could be synchronous. But we typically do this for public functions where we want to reserve the ability to make them asynchronous in the future, and so we want the consuming component to deal with them as an asynchronous function.

But since this is a private function, I wouldn't worry about future-proofing.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@var-const In general, JavaScript is completely single-threaded (there's no way to block other than by spinning the CPU) and so wrapping a computation in a promise doesn't really help (in particular it won't enable any parallelism).

And PersistencePromise is extra weird because it's a specially-designed Promise-like construct that tries to be as synchronous as possible because IndexedDb has weird semantics where in the completion for one operation you must synchronously start the next operation or else your transaction will auto-close. So even using PersistencePromise, this code is actually 100% synchronous. So you can go ahead and just yank PersistencePromise out.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JavaScript is completely single-threaded

Right. Thanks, made this function return the result directly.

docs.forEach((key, localView) => {
for (const batch of batches) {
localView = batch.applyToLocalView(key, localView);
}
results = results.insert(key, localView);
});
resolve(results);
});
}

/**
* Gets the local view of the documents identified by `keys`.
*
Expand All @@ -84,28 +106,40 @@ export class LocalDocumentsView {
transaction: PersistenceTransaction,
keys: DocumentKeySet
): PersistencePromise<MaybeDocumentMap> {
return this.remoteDocumentCache
.getEntries(transaction, keys)
.next(docs => this.getLocalViewOfDocuments(transaction, docs));
}

/**
* Similar to `getDocuments`, but creates the local view from the given
* `baseDocs` without retrieving documents from the local store.
*/
getLocalViewOfDocuments(
transaction: PersistenceTransaction,
baseDocs: NullableMaybeDocumentMap
): PersistencePromise<MaybeDocumentMap> {
let allKeys = documentKeySet();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you don't want to construct a new set just to pass the keys, you could change the signature for getAllMutationBatchesAffectingDocumentKeys to accept a map of DocumentKey to {}. That indicates that you don't care about the values in the map, as you won't have their type, but you will still be able to iterate through the keys. Alternatively, I would consider defining a .keySet() method for SortedMap, since iterating through and simultaneously building up a new immutable set is not very efficient.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done (I had to use any, though, compiler complained about MaybeDocument | null not being assignable to {}).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you try using AnyJs instead of any?

any is dangerous in that it basically opts out of typechecking. So if you type the values as any, TypeScript will let you do anything with the values (call nonexistent methods, etc.) which is the opposite of what we want (we don't want to be able to do anything with the values).

AnyJs is a type we introduced essentially meant to be a supertype of any JS type (similar to Object in java). So TypeScript won't let you do anything with it without doing typeof / instanceof checks to narrow it down.

TypeScript 3.x actually introduced unknown which is similar to AnyJs and I'm hoping we can migrate to it soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, missed this comment. Done now.

baseDocs.forEach(key => {
allKeys = allKeys.add(key);
});

return this.mutationQueue
.getAllMutationBatchesAffectingDocumentKeys(transaction, keys)
.next(batches => {
const promises = [] as Array<PersistencePromise<void>>;
.getAllMutationBatchesAffectingDocumentKeys(transaction, allKeys)
.next(batches =>
this.applyLocalMutationsToDocuments(transaction, baseDocs, batches)
)
.next(docs => {
let results = maybeDocumentMap();
keys.forEach(key => {
promises.push(
this.getDocumentInternal(transaction, key, batches).next(
maybeDoc => {
// TODO(http://b/32275378): Don't conflate missing / deleted.
if (!maybeDoc) {
maybeDoc = new NoDocument(
key,
SnapshotVersion.forDeletedDoc()
);
}
results = results.insert(key, maybeDoc);
}
)
);
docs.forEach((key, maybeDoc) => {
// TODO(http://b/32275378): Don't conflate missing / deleted.
if (!maybeDoc) {
maybeDoc = new NoDocument(key, SnapshotVersion.forDeletedDoc());
}
results = results.insert(key, maybeDoc);
});
return PersistencePromise.waitFor(promises).next(() => results);

return results;
});
}

Expand Down
7 changes: 6 additions & 1 deletion packages/firestore/src/local/local_serializer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,12 @@ export class LocalSerializer {
/** Encodes a document for storage locally. */
toDbRemoteDocument(maybeDoc: MaybeDocument): DbRemoteDocument {
if (maybeDoc instanceof Document) {
const doc = this.remoteSerializer.toDocument(maybeDoc);
let doc: api.Document;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up to you, but this bit could be shortened to:

const doc = maybeDoc.proto ? maybeDoc.proto : this.remoteSerializer.toDocument(maybeDoc);

or even:

const doc = maybeDoc.proto || this.remoteSerializer.toDocument(maybeDoc);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. (I like the Lua-style second version, but chose the first one because it's more similar to other platforms)

if (maybeDoc.proto) {
doc = maybeDoc.proto;
} else {
doc = this.remoteSerializer.toDocument(maybeDoc);
}
const hasCommittedMutations = maybeDoc.hasCommittedMutations;
return new DbRemoteDocument(
/* unknownDocument= */ null,
Expand Down
42 changes: 29 additions & 13 deletions packages/firestore/src/local/local_store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
DocumentKeySet,
documentKeySet,
DocumentMap,
maybeDocumentMap,
MaybeDocumentMap
} from '../model/collections';
import { MaybeDocument } from '../model/document';
Expand Down Expand Up @@ -466,11 +467,18 @@ export class LocalStore {
}
);

let changedDocKeys = documentKeySet();
let changedDocs = maybeDocumentMap();
let updatedKeys = documentKeySet();
remoteEvent.documentUpdates.forEach((key, doc) => {
changedDocKeys = changedDocKeys.add(key);
promises.push(
documentBuffer.getEntry(txn, key).next(existingDoc => {
updatedKeys = updatedKeys.add(key);
});

// Each loop iteration only affects its "own" doc, so it's safe to get all the remote
// documents in advance in a single call.
promises.push(
documentBuffer.getEntries(txn, updatedKeys).next(existingDocs => {
remoteEvent.documentUpdates.forEach((key, doc) => {
const existingDoc = existingDocs.get(key);
// If a document update isn't authoritative, make sure we don't
// apply an old document version to the remote cache. We make an
// exception for SnapshotVersion.MIN which can happen for
Expand All @@ -484,6 +492,7 @@ export class LocalStore {
doc.version.compareTo(existingDoc.version) >= 0
) {
documentBuffer.addEntry(doc);
changedDocs = changedDocs.insert(key, doc);
} else {
log.debug(
LOG_TAG,
Expand All @@ -495,14 +504,18 @@ export class LocalStore {
doc.version
);
}
})
);
if (remoteEvent.resolvedLimboDocuments.has(key)) {
promises.push(
this.persistence.referenceDelegate.updateLimboDocument(txn, key)
);
}
});

if (remoteEvent.resolvedLimboDocuments.has(key)) {
promises.push(
this.persistence.referenceDelegate.updateLimboDocument(
txn,
key
)
);
}
});
})
);

// HACK: The only reason we allow a null snapshot version is so that we
// can synthesize remote events when we get permission denied errors while
Expand Down Expand Up @@ -532,7 +545,10 @@ export class LocalStore {
return PersistencePromise.waitFor(promises)
.next(() => documentBuffer.apply(txn))
.next(() => {
return this.localDocuments.getDocuments(txn, changedDocKeys);
return this.localDocuments.getLocalViewOfDocuments(
txn,
changedDocs
);
});
}
);
Expand Down
47 changes: 46 additions & 1 deletion packages/firestore/src/local/memory_remote_document_cache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,16 @@

import { Query } from '../core/query';
import {
DocumentKeySet,
documentKeySet,
DocumentMap,
documentMap,
DocumentSizeEntry,
DocumentSizeEntries,
MaybeDocumentMap,
maybeDocumentMap
maybeDocumentMap,
NullableMaybeDocumentMap,
nullableMaybeDocumentMap
} from '../model/collections';
import { Document, MaybeDocument, NoDocument } from '../model/document';
import { DocumentKey } from '../model/document_key';
Expand Down Expand Up @@ -107,6 +111,40 @@ export class MemoryRemoteDocumentCache implements RemoteDocumentCache {
return PersistencePromise.resolve(this.docs.get(documentKey));
}

getEntries(
transaction: PersistenceTransaction,
documentKeys: DocumentKeySet
): PersistencePromise<NullableMaybeDocumentMap> {
let results = nullableMaybeDocumentMap();
documentKeys.forEach(documentKey => {
const entry = this.docs.get(documentKey);
results = results.insert(documentKey, entry ? entry.maybeDocument : null);
});
return PersistencePromise.resolve(results);
}

/**
* Looks up several entries in the cache.
*
* @param documentKeys The set of keys entries to look up.
* @return A map of MaybeDocuments indexed by key (if a document cannot be
* found, the key will be mapped to null) and a map of sizes indexed by
* key (zero if the key cannot be found).
*/
getSizedEntries(
transaction: PersistenceTransaction,
documentKeys: DocumentKeySet
): PersistencePromise<DocumentSizeEntries> {
let results = nullableMaybeDocumentMap();
let sizeMap = new SortedMap<DocumentKey, number>(DocumentKey.comparator);
documentKeys.forEach(documentKey => {
const entry = this.docs.get(documentKey);
results = results.insert(documentKey, entry ? entry.maybeDocument : null);
sizeMap = sizeMap.insert(documentKey, entry ? entry.size : 0);
});
return PersistencePromise.resolve({ maybeDocuments: results, sizeMap });
}

getDocumentsMatchingQuery(
transaction: PersistenceTransaction,
query: Query
Expand Down Expand Up @@ -203,4 +241,11 @@ export class MemoryRemoteDocumentChangeBuffer extends RemoteDocumentChangeBuffer
): PersistencePromise<DocumentSizeEntry | null> {
return this.documentCache.getSizedEntry(transaction, documentKey);
}

protected getAllFromCache(
transaction: PersistenceTransaction,
documentKeys: DocumentKeySet
): PersistencePromise<DocumentSizeEntries> {
return this.documentCache.getSizedEntries(transaction, documentKeys);
}
}
Loading