Skip to content

Firestore: Misc bloom filter cleanup ported from Android #7474

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
Merged
merged 1 commit into from
Jul 20, 2023
Merged

Firestore: Misc bloom filter cleanup ported from Android #7474

merged 1 commit into from
Jul 20, 2023

Conversation

dconeybe
Copy link
Contributor

@dconeybe dconeybe commented Jul 18, 2023 •
edited
Loading

In firebase/firebase-android-sdk#5145, the internal bloom filter logic and the testing hooks were improved. This PR ports the relevant changes. There are no net behavior changes, only code cleanup and some minor refactoring.

Some minor changes made in this PR were ported to Android in firebase/firebase-android-sdk#5177 and to the iOS SDK in firebase/firebase-ios-sdk#11679.

@dconeybe dconeybe self-assigned this Jul 18, 2023
@changeset-bot
Copy link

changeset-bot bot commented Jul 18, 2023

⚠️ No Changeset found

Latest commit: 84d928c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@dconeybe dconeybe mentioned this pull request Jul 14, 2023
Merged
@google-oss-bot
Copy link
Contributor

Size Report 1

Affected Products

  • @firebase/firestore

    TypeBase (e91f82a)Merge (16fd721)Diff
    browser371 kB370 kB-67 B (-0.0%)
    esm5356 kB356 kB+49 B (+0.0%)
    main569 kB569 kB+81 B (+0.0%)
    module371 kB370 kB-67 B (-0.0%)
    react-native371 kB371 kB-67 B (-0.0%)

  • bundle

    TypeBase (e91f82a)Merge (16fd721)Diff
    firestore (Persistence)301 kB301 kB+29 B (+0.0%)
    firestore (Query Cursors)239 kB239 kB+29 B (+0.0%)
    firestore (Query)236 kB236 kB+29 B (+0.0%)
    firestore (Read data once)224 kB224 kB+29 B (+0.0%)
    firestore (Realtime updates)226 kB226 kB+29 B (+0.0%)

  • firebase

    TypeBase (e91f82a)Merge (16fd721)Diff
    firebase-compat.js774 kB774 kB+45 B (+0.0%)
    firebase-firestore-compat.js340 kB340 kB+45 B (+0.0%)
    firebase-firestore.js430 kB430 kB-59 B (-0.0%)

Test Logs

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

@google-oss-bot
Copy link
Contributor

Size Analysis Report 1

Affected Products

  • @firebase/firestore

    • enableMultiTabIndexedDbPersistence

      Size

      TypeBase (e91f82a)Merge (16fd721)Diff
      size221 kB221 kB+29 B (+0.0%)
      size-with-ext-deps292 kB292 kB+29 B (+0.0%)

    • getDoc

      Size

      TypeBase (e91f82a)Merge (16fd721)Diff
      size143 kB143 kB+29 B (+0.0%)
      size-with-ext-deps213 kB213 kB+29 B (+0.0%)

    • getDocFromServer

      Size

      TypeBase (e91f82a)Merge (16fd721)Diff
      size143 kB143 kB+29 B (+0.0%)
      size-with-ext-deps213 kB213 kB+29 B (+0.0%)

    • getDocs

      Size

      TypeBase (e91f82a)Merge (16fd721)Diff
      size145 kB145 kB+29 B (+0.0%)
      size-with-ext-deps215 kB215 kB+29 B (+0.0%)

    • getDocsFromServer

      Size

      TypeBase (e91f82a)Merge (16fd721)Diff
      size145 kB145 kB+29 B (+0.0%)
      size-with-ext-deps215 kB215 kB+29 B (+0.0%)

    • onSnapshot

      Size

      TypeBase (e91f82a)Merge (16fd721)Diff
      size145 kB145 kB+29 B (+0.0%)
      size-with-ext-deps216 kB216 kB+29 B (+0.0%)

    • onSnapshotsInSync

      Size

      TypeBase (e91f82a)Merge (16fd721)Diff
      size135 kB135 kB+29 B (+0.0%)
      size-with-ext-deps205 kB205 kB+29 B (+0.0%)

    • persistentMultipleTabManager

      Size

      TypeBase (e91f82a)Merge (16fd721)Diff
      size217 kB217 kB+29 B (+0.0%)
      size-with-ext-deps289 kB289 kB+29 B (+0.0%)

Test Logs

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

dconeybe added a commit to firebase/firebase-android-sdk that referenced this pull request Jul 18, 2023
@dconeybe
Port some small code changes from firebase/firebase-js-sdk#7474 to be…
5d9bfd1 
… consistent with the web sdk
@dconeybe dconeybe mentioned this pull request Jul 18, 2023
Merged
@dconeybe dconeybe marked this pull request as ready for review July 19, 2023 17:57
@dconeybe dconeybe requested review from a team as code owners July 19, 2023 17:57
@dconeybe dconeybe requested a review from milaGGL July 19, 2023 17:57
milaGGL
milaGGL approved these changes Jul 19, 2023
View reviewed changes
Copy link
Contributor

@milaGGL milaGGL left a comment

Choose a reason for hiding this comment

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

Looks better with parseBloomFilter function. good job!

@dconeybe dconeybe merged commit 3b4171d into master Jul 20, 2023
@dconeybe dconeybe deleted the dconeybe/BloomFilterApplyCleanup branch July 20, 2023 02:29
dconeybe added a commit to firebase/firebase-android-sdk that referenced this pull request Jul 20, 2023
@dconeybe
Port some small code changes from firebase/firebase-js-sdk#7474 to be…
965ac2f 
… consistent with the web sdk (#5177)
dconeybe added a commit to firebase/firebase-ios-sdk that referenced this pull request Jul 24, 2023
@dconeybe
testBloomFilterShouldCorrectlyEncodeComplexUnicodeCharacters started …
d5ec301 
…(porting from firebase/firebase-js-sdk#7412 and firebase/firebase-js-sdk#7413 and firebase/firebase-js-sdk#7474)
@dconeybe dconeybe mentioned this pull request Aug 15, 2023
Merged
@firebase firebase locked and limited conversation to collaborators Aug 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@milaGGL milaGGL milaGGL approved these changes

Assignees

@dconeybe dconeybe

Labels
api: firestore
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dconeybe @google-oss-bot @milaGGL