Skip to content
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

Remove reliance on requestTracker in loadGraphs #9440

Merged
merged 15 commits into from
Dec 15, 2023

Conversation

irismoini
Copy link
Contributor

@irismoini irismoini commented Dec 14, 2023

↪️ Pull Request

This PR changes the cache keys for the requestGraph, bundleGraph, and assetGraph by appending their corresponding names to the keys. This makes it easy to identify the requestGraph, bundleGraph, and assetGraph files in the cache directory which is useful for Parcel-query's loadGraph function. This PR modifies the loadGraphs function in Parcel-query to to load the bundleGraph and assetGraph without needing information from the requestTrackerto identitfy these graphs, thus removing the reliance on the requestTracker.

This PR is to set us up for future work where only necessary graphs are deserialized for a given Parcel query, i.e. getBundles does not require the assetGraph and thus there's no need to spend time deserializing it.

Note: bundleInfo still requires the requestTracker. (No easy way to retrieve it outside of requestTracker).

💻 Examples

For example a cache key for the requestGraph that previously would have be represented like f067ed41c9e7c588, is now represented as f067ed41c9e7c588-RequestGraph. Same for the bundleGraph and assetGraph cache keys.

Previously if an error occurred with deserializing or indentifying the requestTracker, the bundleGraph and assetGraph could not be fetched and queries like inspectCache would fail completely. Now you will get an output like the following:

Request Graph could not be found
╔═══════════════════════════════════════════════════════════════════════════════════╗
║                                    Cache Info                                     ║
╟────────────────────┬────────────────────┬────────────────────┬────────────────────╢
║ Graphs             │ Size (bytes)       │ Deserialize (ms)   │ Serialize (ms)     ║
╟────────────────────┼────────────────────┼────────────────────┼────────────────────╢
║ BundleGraph        │ 1270350344         │ 5378               │ 2821               ║
╟────────────────────┼────────────────────┼────────────────────┼────────────────────╢
║ AssetGraph         │ 621551347          │ 5020               │ 2350               ║
╟────────────────────┼────────────────────┼────────────────────┼────────────────────╢
║ Totals             │ 1891901691         │ 10398              │ 5171               ║
╚════════════════════╧════════════════════╧════════════════════╧════════════════════╝

🚨 Test instructions

Tested manually with different queries and by inspecting the cache.

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

@irismoini irismoini self-assigned this Dec 14, 2023
@irismoini irismoini changed the title Load bundleGraph and AssetGraph directly from cache in Parcel query. Remove reliance on requestTracker in loadGraphs Dec 14, 2023
@irismoini irismoini merged commit 9b66e25 into v2 Dec 15, 2023
15 of 16 checks passed
@irismoini irismoini deleted the imoini/P2X-1172-remove-reliance-on-requestTracker branch December 15, 2023 02:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants