Skip to content

Reduce memory usage by applying query check sooner #4591

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

Merged
merged 11 commits into from
Jan 30, 2023

Conversation

wu-hui
Copy link
Contributor

@wu-hui wu-hui commented Jan 25, 2023

b/263835797 for internal tracking

@github-actions
Copy link
Contributor

github-actions bot commented Jan 25, 2023

Javadoc Changes:
--- /home/runner/diff/original/firebase-kotlindoc/android/com/google/firebase/firestore/AggregateQuery.html	2023-01-30 19:32:26.046202084 +0000
+++ /home/runner/diff/modified/firebase-kotlindoc/android/com/google/firebase/firestore/AggregateQuery.html	2023-01-30 19:31:23.330124939 +0000
@@ -47,7 +47,7 @@
           <tr>
             <td width="40%"><code>@<a href="https://developer.android.com/reference/kotlin/androidx/annotation/NonNull.html">NonNull</a> <a href="https://developers.google.com/android/reference/com/google/android/gms/tasks/Task.html">Task</a>&lt;<a href="/docs/reference/android/com/google/firebase/firestore/AggregateQuerySnapshot.html">AggregateQuerySnapshot</a>&gt;</code></td>
             <td>
-              <div><code><a href="/docs/reference/android/com/google/firebase/firestore/AggregateQuery.html#get(com.google.firebase.firestore.AggregateSource)">get</a>(@<a href="https://developer.android.com/reference/kotlin/androidx/annotation/NonNull.html">NonNull</a> <a href="https://developer.android.com/reference/kotlin/java/lang/Object.html">Object</a>&nbsp;source)</code></div>
+              <div><code><a href="/docs/reference/android/com/google/firebase/firestore/AggregateQuery.html#get(com.google.firebase.firestore.AggregateSource)">get</a>(@<a href="https://developer.android.com/reference/kotlin/androidx/annotation/NonNull.html">NonNull</a> <a href="/docs/reference/android/com/google/firebase/firestore/AggregateSource.html">AggregateSource</a>&nbsp;source)</code></div>
               <p>Executes this query.</p>
             </td>
           </tr>
@@ -122,7 +122,7 @@
     </div>
     <div><a name="get-com.google.firebase.firestore.AggregateSource-"></a><a name="get"></a>
       <h3 class="api-name" id="get(com.google.firebase.firestore.AggregateSource)">get</h3>
-      <pre class="api-signature no-pretty-print">public&nbsp;@<a href="https://developer.android.com/reference/kotlin/androidx/annotation/NonNull.html">NonNull</a> <a href="https://developers.google.com/android/reference/com/google/android/gms/tasks/Task.html">Task</a>&lt;<a href="/docs/reference/android/com/google/firebase/firestore/AggregateQuerySnapshot.html">AggregateQuerySnapshot</a>&gt;&nbsp;<a href="/docs/reference/android/com/google/firebase/firestore/AggregateQuery.html#get(com.google.firebase.firestore.AggregateSource)">get</a>(@<a href="https://developer.android.com/reference/kotlin/androidx/annotation/NonNull.html">NonNull</a> <a href="https://developer.android.com/reference/kotlin/java/lang/Object.html">Object</a>&nbsp;source)</pre>
+      <pre class="api-signature no-pretty-print">public&nbsp;@<a href="https://developer.android.com/reference/kotlin/androidx/annotation/NonNull.html">NonNull</a> <a href="https://developers.google.com/android/reference/com/google/android/gms/tasks/Task.html">Task</a>&lt;<a href="/docs/reference/android/com/google/firebase/firestore/AggregateQuerySnapshot.html">AggregateQuerySnapshot</a>&gt;&nbsp;<a href="/docs/reference/android/com/google/firebase/firestore/AggregateQuery.html#get(com.google.firebase.firestore.AggregateSource)">get</a>(@<a href="https://developer.android.com/reference/kotlin/androidx/annotation/NonNull.html">NonNull</a> <a href="/docs/reference/android/com/google/firebase/firestore/AggregateSource.html">AggregateSource</a>&nbsp;source)</pre>
       <p>Executes this query.</p>
       <div class="devsite-table-wrapper">
         <table class="responsive">
@@ -133,7 +133,7 @@
           </thead>
           <tbody class="list">
             <tr>
-              <td width="40%"><code>@<a href="https://developer.android.com/reference/kotlin/androidx/annotation/NonNull.html">NonNull</a> <a href="https://developer.android.com/reference/kotlin/java/lang/Object.html">Object</a>&nbsp;source</code></td>
+              <td width="40%"><code>@<a href="https://developer.android.com/reference/kotlin/androidx/annotation/NonNull.html">NonNull</a> <a href="/docs/reference/android/com/google/firebase/firestore/AggregateSource.html">AggregateSource</a>&nbsp;source</code></td>
               <td>
                 <p>The source from which to acquire the aggregate results.</p>
               </td>
--- /home/runner/diff/original/firebase-kotlindoc/kotlin/com/google/firebase/firestore/AggregateQuery.html	2023-01-30 19:32:26.086202147 +0000
+++ /home/runner/diff/modified/firebase-kotlindoc/kotlin/com/google/firebase/firestore/AggregateQuery.html	2023-01-30 19:31:23.358124994 +0000
@@ -47,7 +47,7 @@
           <tr>
             <td width="40%"><code><a href="https://developers.google.com/android/reference/com/google/android/gms/tasks/Task.html">Task</a>&lt;<a href="/docs/reference/kotlin/com/google/firebase/firestore/AggregateQuerySnapshot.html">AggregateQuerySnapshot</a>!&gt;</code></td>
             <td>
-              <div><code><a href="/docs/reference/kotlin/com/google/firebase/firestore/AggregateQuery.html#get(com.google.firebase.firestore.AggregateSource)">get</a>(source:&nbsp;<a href="https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-any/index.html">Any</a>)</code></div>
+              <div><code><a href="/docs/reference/kotlin/com/google/firebase/firestore/AggregateQuery.html#get(com.google.firebase.firestore.AggregateSource)">get</a>(source:&nbsp;<a href="/docs/reference/kotlin/com/google/firebase/firestore/AggregateSource.html">AggregateSource</a>)</code></div>
               <p>Executes this query.</p>
             </td>
           </tr>
@@ -122,7 +122,7 @@
     </div>
     <div><a name="get-com.google.firebase.firestore.AggregateSource-"></a><a name="get"></a>
       <h3 class="api-name" id="get(com.google.firebase.firestore.AggregateSource)">get</h3>
-      <pre class="api-signature no-pretty-print">fun&nbsp;<a href="/docs/reference/kotlin/com/google/firebase/firestore/AggregateQuery.html#get(com.google.firebase.firestore.AggregateSource)">get</a>(source:&nbsp;<a href="https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-any/index.html">Any</a>):&nbsp;<a href="https://developers.google.com/android/reference/com/google/android/gms/tasks/Task.html">Task</a>&lt;<a href="/docs/reference/kotlin/com/google/firebase/firestore/AggregateQuerySnapshot.html">AggregateQuerySnapshot</a>!&gt;</pre>
+      <pre class="api-signature no-pretty-print">fun&nbsp;<a href="/docs/reference/kotlin/com/google/firebase/firestore/AggregateQuery.html#get(com.google.firebase.firestore.AggregateSource)">get</a>(source:&nbsp;<a href="/docs/reference/kotlin/com/google/firebase/firestore/AggregateSource.html">AggregateSource</a>):&nbsp;<a href="https://developers.google.com/android/reference/com/google/android/gms/tasks/Task.html">Task</a>&lt;<a href="/docs/reference/kotlin/com/google/firebase/firestore/AggregateQuerySnapshot.html">AggregateQuerySnapshot</a>!&gt;</pre>
       <p>Executes this query.</p>
       <div class="devsite-table-wrapper">
         <table class="responsive">
@@ -133,7 +133,7 @@
           </thead>
           <tbody class="list">
             <tr>
-              <td width="40%"><code>source:&nbsp;<a href="https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-any/index.html">Any</a></code></td>
+              <td width="40%"><code>source:&nbsp;<a href="/docs/reference/kotlin/com/google/firebase/firestore/AggregateSource.html">AggregateSource</a></code></td>
               <td>
                 <p>The source from which to acquire the aggregate results.</p>
               </td>

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jan 25, 2023

Coverage Report 1

Affected Products

  • firebase-firestore

    Overall coverage changed from 44.53% (9d99981) to 44.52% (23b3416) by -0.01%.

    FilenameBase (9d99981)Merge (23b3416)Diff
    LocalStore.java100.00%99.37%-0.63%
    LruGarbageCollector.java97.20%93.46%-3.74%
    MemoryRemoteDocumentCache.java98.21%98.28%+0.06%
    SetMutation.java94.44%97.22%+2.78%
    SQLiteRemoteDocumentCache.java98.17%98.21%+0.05%

Test Logs

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

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jan 25, 2023

@github-actions
Copy link
Contributor

github-actions bot commented Jan 25, 2023

Unit Test Results

   158 files  ±0     158 suites  ±0   2m 10s ⏱️ -37s
1 116 tests +4  1 100 ✔️ +4  16 💤 ±0  0 ±0 
2 232 runs  +8  2 200 ✔️ +8  32 💤 ±0  0 ±0 

Results for commit d55d2a5. ± Comparison against base commit 9d99981.

♻️ This comment has been updated with latest results.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jan 25, 2023

Startup Time Report 1

Note: Layout is sometimes suboptimal due to limited formatting support on GitHub. Please check this report on GCS.

Notes

Startup Times

  • timeToInitialDisplay

    DeviceStatisticsDistributions
    oriole-32
    Percentile9d9998123b3416DiffSignificant (?)
    p10159 ±3 ms162 ±2 ms+2.81 ms (+1.8%)NO
    p25164 ±3 ms167 ±3 ms+2.88 ms (+1.8%)NO
    p50170 ±4 ms173 ±3 ms+3.24 ms (+1.9%)NO
    p75177 ±4 ms181 ±2 ms+3.85 ms (+2.2%)NO
    p90183 ±5 ms191 ±3 ms+8.20 ms (+4.5%)NO

    20 test runs in comparison
    CommitTest Runs
    9d99981
    • 2023-01-26_15:33:48.602371_aiVH
    • 2023-01-26_15:33:48.606248_oPUS
    • 2023-01-26_15:33:48.606275_cPXP
    • 2023-01-26_15:33:48.606283_ADWz
    • 2023-01-26_15:33:48.606291_tViN
    • 2023-01-26_15:33:48.606299_cBaM
    • 2023-01-26_15:33:48.606306_DQPz
    • 2023-01-26_15:33:48.606314_hDiH
    • 2023-01-26_15:33:48.606320_qsFY
    • 2023-01-26_15:33:48.606327_Dyui
    23b3416
    • 2023-01-30_19:38:30.063618_tIsR
    • 2023-01-30_19:38:30.064575_DGVd
    • 2023-01-30_19:38:30.064584_HXek
    • 2023-01-30_19:38:30.064589_AfPi
    • 2023-01-30_19:38:30.064595_ExVf
    • 2023-01-30_19:38:30.064600_udhj
    • 2023-01-30_19:38:30.064606_MznX
    • 2023-01-30_19:38:30.064611_CWbo
    • 2023-01-30_19:38:30.064620_ShqR
    • 2023-01-30_19:38:30.064625_Iyth
    redfin-30
    Percentile9d9998123b3416DiffSignificant (?)
    p10185 ±3 ms206 ±2 ms+20.9 ms (+11.3%)YES
    p25191 ±2 ms211 ±3 ms+20.3 ms (+10.6%)YES
    p50197 ±4 ms218 ±3 ms+21.1 ms (+10.7%)YES
    p75205 ±4 ms225 ±3 ms+20.5 ms (+10.0%)YES
    p90212 ±4 ms234 ±5 ms+21.7 ms (+10.2%)MAYBE

    20 test runs in comparison
    CommitTest Runs
    9d99981
    • 2023-01-26_15:33:48.602371_aiVH
    • 2023-01-26_15:33:48.606248_oPUS
    • 2023-01-26_15:33:48.606275_cPXP
    • 2023-01-26_15:33:48.606283_ADWz
    • 2023-01-26_15:33:48.606291_tViN
    • 2023-01-26_15:33:48.606299_cBaM
    • 2023-01-26_15:33:48.606306_DQPz
    • 2023-01-26_15:33:48.606314_hDiH
    • 2023-01-26_15:33:48.606320_qsFY
    • 2023-01-26_15:33:48.606327_Dyui
    23b3416
    • 2023-01-30_19:38:30.063618_tIsR
    • 2023-01-30_19:38:30.064575_DGVd
    • 2023-01-30_19:38:30.064584_HXek
    • 2023-01-30_19:38:30.064589_AfPi
    • 2023-01-30_19:38:30.064595_ExVf
    • 2023-01-30_19:38:30.064600_udhj
    • 2023-01-30_19:38:30.064606_MznX
    • 2023-01-30_19:38:30.064611_CWbo
    • 2023-01-30_19:38:30.064620_ShqR
    • 2023-01-30_19:38:30.064625_Iyth

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

Copy link
Contributor

@ehsannas ehsannas left a comment

Choose a reason for hiding this comment

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

Generally LGTM. A couple of minor improvements suggested below.

@ehsannas ehsannas assigned wu-hui and unassigned ehsannas Jan 27, 2023
Copy link
Contributor Author

@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.

Thanks.

@wu-hui wu-hui merged commit 96ef850 into master Jan 30, 2023
@wu-hui wu-hui deleted the wuandy/ApplyQueryToRemoteDocRead branch January 30, 2023 22:50
@firebase firebase locked and limited conversation to collaborators Mar 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants