WorryFree Computers   »   [go: up one dir, main page]
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

Add option to allow SDK create cache indexes automatically to improve query execution locally #11596

Merged
merged 14 commits into from
Aug 29, 2023
Merged

Add option to allow SDK create cache indexes automatically to improve query execution locally #11596

merged 14 commits into from
Aug 29, 2023

Conversation

cherylEnkidu
Copy link
Contributor
@cherylEnkidu cherylEnkidu commented Jul 24, 2023 •
edited

Preparing work for index auto creation feature.
#no-changelog

@firebase firebase deleted a comment from google-oss-bot Jul 24, 2023
@google-oss-bot
Copy link
google-oss-bot commented Jul 24, 2023 •
edited

Size Report 1

Affected Products

  • FirebaseFirestore

    TypeBase (3df15ba)Merge (a3ff892)Diff
    CocoaPods?-51.5 kB? (?)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/NT3XGjxxDM.html

@cherylEnkidu cherylEnkidu force-pushed the cheryllin/autoIndexing branch 4 times, most recently from 9c0f65d to 3f248b8 Compare July 26, 2023 19:41
@google-oss-bot
Copy link
google-oss-bot commented Jul 26, 2023 •
edited

Coverage Report 1

Affected Products

  • FirebaseFirestore-iOS-FirebaseFirestore.framework

    Overall coverage changed from 88.12% (2e7eecd) to 88.14% (5180393) by +0.02%.

    17 individual files with coverage change

    FilenameBase (2e7eecd)Merge (5180393)Diff
    exception.cc84.21%23.68%-60.53%
    firestore.cc92.13%92.41%+0.28%
    firestore_client.cc98.91%98.94%+0.03%
    FIRFirestore.mm88.70%88.97%+0.27%
    FIRPersistentCacheIndexManager.mm?100.00%?
    leveldb_index_manager.cc97.67%97.72%+0.06%
    leveldb_key.cc98.14%98.82%+0.69%
    leveldb_opener.cc76.81%78.99%+2.17%
    leveldb_persistence.cc90.82%92.31%+1.49%
    leveldb_remote_document_cache.cc96.41%96.55%+0.14%
    local_documents_view.cc96.86%96.92%+0.06%
    memory_index_manager.cc50.00%51.61%+1.61%
    memory_persistence.cc100.00%97.37%-2.63%
    memory_remote_document_cache.cc93.51%93.83%+0.32%
    persistent_cache_index_manager.cc?100.00%?
    query_engine.cc98.28%98.68%+0.40%
    target_index_matcher.cc97.80%95.28%-2.53%

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/DMi2ycmIRI.html

@cherylEnkidu cherylEnkidu force-pushed the cheryllin/autoIndexing branch 2 times, most recently from e1250c4 to 7c8e0ac Compare July 30, 2023 02:08
wu-hui
wu-hui reviewed Aug 18, 2023
View reviewed changes
Firestore/core/src/local/query_engine.cc Outdated Show resolved Hide resolved
Firestore/core/src/model/target_index_matcher.cc Show resolved Hide resolved
Firestore/core/src/local/persistence.h Outdated Show resolved Hide resolved
Firestore/core/src/local/leveldb_transaction.h Outdated Show resolved Hide resolved
Firestore/core/src/local/leveldb_transaction.h Outdated Show resolved Hide resolved
Firestore/core/src/local/leveldb_transaction.cc Outdated Show resolved Hide resolved
wu-hui
wu-hui reviewed Aug 25, 2023
View reviewed changes
Copy link
Contributor
@wu-hui wu-hui left a comment

Choose a reason for hiding this comment

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

Half way through, l reviewed all the non-tests, there are some minor changes requested.

Firestore/core/src/local/index_manager.h Outdated Show resolved Hide resolved
Firestore/Source/API/FIRPersistentCacheSettings+Internal.h Outdated Show resolved Hide resolved
Firestore/Example/Tests/Integration/API/FIRIndexingTests.mm Show resolved Hide resolved
Firestore/Example/Tests/Integration/API/FIRIndexingTests.mm Show resolved Hide resolved
@cherylEnkidu cherylEnkidu removed their assignment Aug 28, 2023
wu-hui
wu-hui approved these changes Aug 28, 2023
View reviewed changes
Copy link
Contributor
@wu-hui wu-hui left a comment

Choose a reason for hiding this comment

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

Couple more small nits, I am approving it however.

Firestore/core/test/unit/model/target_index_matcher_test.cc Outdated Show resolved Hide resolved
Firestore/core/test/unit/local/leveldb_local_store_test.cc Outdated Show resolved Hide resolved
@cherylEnkidu cherylEnkidu merged commit c39ef81 into master Aug 29, 2023
42 checks passed
@cherylEnkidu cherylEnkidu deleted the cheryllin/autoIndexing branch August 29, 2023 15:26
@mikehardy
Copy link
Contributor

This looks neat! Was checking documentation for possible integration in react-native-firebase and I may be wrong, but is the documentation for the API in sync?

I see the API in use here:
https://github.com/firebase/firebase-ios-sdk/pull/11596/files#diff-9f6f97afe1406491a0af86b5751973e5908e45e73ee5a698445157b6e67f0f9eR172-R191

And I see a mention of the API here as the thing to use in future: https://firebase.google.com/docs/reference/ios/firebasefirestore/api/reference/Classes/FIRFirestore#-setindexconfigurationfromjson:completion:

But I'm unable to find docs on the web for this object PersistentCacheIndexManager or it's methods

andrewheard pushed a commit that referenced this pull request Sep 20, 2023
@cherylEnkidu @andrewheard
Add option to allow SDK create cache indexes automatically to improve…
eb2404e 
… query execution locally (#11596)
@firebase firebase locked and limited conversation to collaborators Sep 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@wu-hui wu-hui wu-hui approved these changes

Assignees

@wu-hui wu-hui

Labels
api: firestore
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@cherylEnkidu @google-oss-bot @mikehardy @wu-hui