perf: Cache installationId lookup in ParseClient.buildHeaders#1
Open
AdrianCurtin wants to merge 3 commits into
Open
perf: Cache installationId lookup in ParseClient.buildHeaders#1AdrianCurtin wants to merge 3 commits into
installationId lookup in ParseClient.buildHeaders#1AdrianCurtin wants to merge 3 commits into
Conversation
Add ParseInstallation.currentInstallationId() that returns a cached installationId (or reads it from the local store / creates an installation if missing). Update ParseClient to call currentInstallationId() instead of loading and JSON-decoding the full ParseInstallation on every HTTP request to avoid repeated local-store reads and improve performance; _createInstallation still populates the cached value.
There was a problem hiding this comment.
Pull request overview
This PR reduces per-request overhead in the Dart SDK by caching the Parse installation UUID used for the X-Parse-Installation-Id header, avoiding repeated local-store reads/JSON decoding on hot HTTP paths introduced by parse-community#1140.
Changes:
- Added
ParseInstallation.currentInstallationId()to cache and return the installation UUID. - Updated
ParseClient.buildHeadersto usecurrentInstallationId()instead of loading the fullParseInstallation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/dart/lib/src/objects/parse_installation.dart | Adds a cache-first helper for retrieving the installation UUID. |
| packages/dart/lib/src/network/parse_client.dart | Switches header construction to use the cached installation UUID helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address Copilot review feedback: - Skip constructing a full ParseInstallation when only the UUID is needed; read keyInstallationId straight from the stored JSON map. - Clear the cache if bootstrap persistence fails so the next call retries, matching the pre-cache retry semantics. - Add @VisibleForTesting debugResetInstallationIdCache() so tests and apps that clear core storage mid-session can invalidate the static cache.
Add validation for installationId and update install handling/tests. - Introduce _isUsableInstallationId to centralize validation (non-null, String, non-whitespace). - Ensure cached _currentInstallationId and values read from local store are validated and treated as missing when invalid. - Reject stored installations with missing/empty/whitespace or non-string installationId in _getFromLocalStore (with debug logging). - Re-stage a usable installationId into _unsavedChanges in _updateInstallation when objectId == null so create() POST bodies include the installationId. - Ensure _createInstallation generates a fresh UUID when cache is empty/invalid instead of reusing poisoned values. - Tests: initialize Parse once, clear store/cache between tests, add regression tests to verify re-staging behavior and that empty/whitespace/non-string installationIds in local store are rejected. These changes prevent corrupted local state (e.g. installationId == "" or whitespace) from being reused in headers or API request bodies and avoid creating server rows with missing installationId.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacks on top of parse-community#1140.
Issue
After parse-community#1140,
ParseClient.buildHeadersruns on every outgoing HTTP request and callsParseInstallation.currentInstallation(), which in turn:keyParseStoreInstallationfrom the local store (coreStore.getString),json.decodes the stored installation document,ParseInstallationinstance withfromJson(allocates dirty-tracking maps),…just to read a single immutable field —
installationId. On hot paths like paginated query loops or batched saves, this is unnecessary CPU and GC pressure on every request, and real disk I/O for stores likeCoreStoreSembast.Approach
ParseInstallation.currentInstallationId(), a static cache-first helper that returns just the install UUID. Populates from the local store on first call (or bootstraps a fresh installation if none exists) and stores the result in the existing_currentInstallationIdstatic. Subsequent calls are a single static-field read.ParseClient.buildHeadersnow callscurrentInstallationId()instead ofcurrentInstallation().installationId.Safety
null, so the cache repopulates from storage on the first call. Correct across launches.nulland may both read the store, but they converge on the same cached value. Worst-case is a redundantgetString— no correctness issue._createInstallationalready guards against duplicate UUID generation via??=.currentInstallation()is unchanged — callers that need the full document (e.g. push setup,save(), channel subscriptions) still work the same way.Tests
All 224 existing tests pass, including the per-verb wiring tests you added in this PR. Happy to add a dedicated cache-hit test if you'd like.