Skip to content

New persistentLocalCache is 20x slower than deprecated enableIndexedDbPersistence #7347

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

Closed
ScottKitchell opened this issue Jun 3, 2023 · 6 comments

Comments

@ScottKitchell
Copy link

Operating System

MacOS 13.3.1

Browser Version

Chrome 113.0.5672.126

Firebase SDK Version

19.22.1

Firebase SDK Product:

Firestore

Describe your project's tooling

SolidJS PWA app with Vite.

Describe the problem

When configuring Firestore with persistence (single tab or multi-tab) in the new recommended way using persistentLocalCache it is 20x slower to query data from the cache than when previously using enableIndexedDbPersistence (now marked as deprecated).

// With enableIndexedDbPersistence (deprecated)
const db = initializeFirestore(app, {})
await enableIndexedDbPersistence(db)
const snapshot = await getDocsFromCache(dataQuery) // 40ms 🙂
// With persistentLocalCache
const db = initializeFirestore(app, {
  localCache: persistentLocalCache({ tabManager: persistentSingleTabManager({}) }),
})
const snapshot = await getDocsFromCache(dataQuery) // 850ms 😱

Persistence config changes were introduced in #7015.

Steps and code to reproduce issue

  1. Start with a firestore database with one collection containing one document.
  2. Create a firestore instance with persistence using enableIndexedDbPersistence. e.g.:
const db = initializeFirestore(app, {})
await enableIndexedDbPersistence(db)
  1. Query the data to ensure the cache contains the firestore document added.
  2. Query the cache with getDocsFromCache and note the time taken (~40ms for me). e.g.:
console.time("getDocsFromCache")
const snapshot = await getDocsFromCache(dataQuery)
console.timeEnd("getDocsFromCache")
  1. Now change the initializeFirestore to use persistentLocalCache. E.g.:
const db = initializeFirestore(app, {
  localCache: persistentLocalCache({ tabManager: persistentSingleTabManager({}) }),
})
  1. Run the same getDocsFromCache query as above and note the time taken (~850ms for me).
  2. Compare times for both queries.
@wu-hui
Copy link
Contributor

wu-hui commented Jun 5, 2023

Thanks for trying it out and reporting the issue! I will spend some time trying to understand this.

@wu-hui
Copy link
Contributor

wu-hui commented Jun 6, 2023

Hi @ScottKitchell

I have tried to reproduce this behavior and failed..the two configurations lead to same query-from-cache performance for me on a 5000 document read, they are both around 140ms in my tests.

I did identify a bug where localCache: persistentCache() can be ignored if you later call connectFirestoreEmulator, but that should lead to a faster query speed not slower so I guess this is not the cause for what you are seeing.

I'd like to ask you to provide a minimum reproduction so I can proceed investigation and better understand your usage pattern. If you do create such a repro, you can share from a github repo. Remember not to include your project config, you can either leave it blank, provide some dummy value or simply make your repro working against a local firestore emulator.

Looking forward to your reply!

@garrett-wombat
Copy link

garrett-wombat commented Jun 9, 2023

@wu-hui The issue with localCache settings being ignored when connected to emulator. Is that in the pipline to be fixe or is that now linked to this issue. Im running into that issue right now. Is a new issue needed to track that?

@ScottKitchell
Copy link
Author

Thanks for looking into it @wu-hui.

Regarding the cache performance, I originally didn't account for the time taken to run enableIndexedDbPersistence, and so when await enableIndexedDbPersistence(db) is included within the time taken to load from the cache then the results are similar.

So TLDR: I can't replicate it.

After changing to use persistentLocalCache it seemed slower to users and myself which got me looking into the performance. The real world query included a where and an orderBy timestamp which I thought might affect it, but when testing these still seemed to be similar across both setup approaches.


What I used to (try to) replicate:

import { FirebaseOptions, initializeApp } from 'firebase/app'
import { getAuth } from 'firebase/auth'
import { collection, enableIndexedDbPersistence, getDocsFromCache, initializeFirestore, persistentLocalCache, getDocs, query, where, orderBy, Firestore, addDoc, Timestamp, writeBatch, doc} from 'firebase/firestore'


// Initialize Firebase

const config: FirebaseOptions = { } // TODO
export const app = initializeApp(config)
export const auth = getAuth(app)


// Create some documents (posts) to query

// {
//   const db = initializeFirestore(app, { localCache: persistentLocalCache() })
//   const batch = writeBatch(db)
//   for(let i = 0; i < 1000; i++) {
//     const postDoc = doc(db, "posts", i.toString())
//     batch.set(postDoc, {
//       text: `post ${i}`,
//       createdAt: Timestamp.fromMillis(1686552899120 + (i * 1000)),
//     })
//   }
//   await batch.commit()
// }


// A function to test the performance of the cache

const queryNTimes = 500

async function testCachePerf(getDb: () => Promise<Firestore>): Promise<number> {
  const start = performance.now()
  const db = await getDb()
  for (let i = 0; i < queryNTimes; i++) {
    await getDocsFromCache(query(
      collection(db, 'posts'),
      orderBy('createdAt', 'desc')
    ))
  }
  const end = performance.now()
  return Math.round((end - start) / queryNTimes)
}


// The different firestore setup approaches to test
// Uncomment one of these at a time to test

const loading = testCachePerf(async () => {
  const db = initializeFirestore(app, { localCache: persistentLocalCache() })
  return db
})
// OR
// const loading = testCachePerf(async () => {
//   const db = initializeFirestore(app, { })
//   await enableIndexedDbPersistence(db)
//   return db
// })


// Render the results

loading.then((timeToLoad) => {
  const perfDetails = `Firestore cache loaded in ${timeToLoad}ms (avg) for ${queryNTimes} queries`
  console.log(perfDetails)
  document.querySelector('#app')!.innerHTML = `<h2>${perfDetails}</h2>`
})

document.querySelector('#app')!.innerHTML = `<h2>Loading...</h2>`

@wu-hui
Copy link
Contributor

wu-hui commented Jun 12, 2023

@garrett-wombat Please see: #7360 and yes we will fix this.

@ScottKitchell The performance difference you reported in the original post looks very similar to the performance difference between memory cache and indexeddb cache (indexeddb being slower than memory). Maybe you mis-configured earlier? Anyways, if no objection, I will close this issue later today.

@ScottKitchell
Copy link
Author

Thanks for your help @wu-hui. No worries, feel free to close.

@wu-hui wu-hui closed this as completed Jun 13, 2023
@firebase firebase locked and limited conversation to collaborators Jul 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants