firestore: only pay .stack cost in error paths - #9102
Conversation
`.stack` is relatively expensive to compute and currently every RPC in Firestore captures the stack, although it's only used in the error cases. We can improve the performance in V8 by deferring source map resolution to only error cases.
product-auto-label
Bot
added
the
api: firestore
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors how callsite error stacks are captured across several Firestore files by storing the Error object itself (as callsiteError) and deferring the .stack access. However, a critical compilation error was introduced in bulk-writer.ts where a reference to the old stack variable was missed in a catch block.
|
This is relatively small change but makes noticable performance improvements for us, as it takes nodejs a dozen milliseconds or so to resolve stacktraces through our sourcemaps in our large node binary and we run with --enable_source_maps in production cc: @MarkDuckworth |
|
/gcbrun |
|
Thank you! BTW I also have another small PR that prevents a crash I observed in production this morning if you have a moment 🙏 |
|
/gcbrun |
3 participants
.stackis relatively expensive to compute and currently every RPC inFirestore captures the stack, although it's only used in the error cases.
We can improve the performance in V8 by deferring source map resolution
to only error cases.