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

Resolve using list instead of get #3726

Merged
merged 2 commits into from
Jun 30, 2023
Merged

Conversation

alexweininger
Copy link
Member

Fixes #3723

Making a HTTP request for each resource is really hurting performance. Ideally we can delay these requests until the user expands the resource or invokes a command on it, but for now I think this is the best solution.

The cache only valid for 3 seconds, long enough to make sure the entire batch of resolve calls can hit the cache. I didn't do anything fancy since I want to validate with the customer that this speeds things up. I figure we can make a util and share this across extensions later.

@alexweininger alexweininger requested a review from a team as a code owner June 23, 2023 19:15
Copy link
Member

@nturinski nturinski left a comment

Choose a reason for hiding this comment

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

Actually, you never actually clear the cache. I'm worried that if we delete a function app, the siteCache will still be in the map because we're doing a list, and then a for each over the returned sites list.

We could just save the array instead and delete that whenever we reset the cache? It might be easier to maintain, though that's not necessarily better than a map or anything.

@alexweininger
Copy link
Member Author

Actually, you never actually clear the cache. I'm worried that if we delete a function app, the siteCache will still be in the map because we're doing a list, and then a for each over the returned sites list.

Good catche 😄 , it's probably a good idea to clear the cache.

I do want to note that technically delete would work OK since RGs will always re-list the resources and only resolve what it knows about. So having deleted things in the cache wouldn't cause issues for that particular case.

I do think it might cause issues at other times, so it's still a good idea.

@alexweininger
Copy link
Member Author

Since the release is delayed maybe we merge this and do another round of testing.

@alexweininger alexweininger merged commit 73624dd into main Jun 30, 2023
@alexweininger alexweininger deleted the alex/optimize-resolver branch June 30, 2023 20:09
@microsoft microsoft locked and limited conversation to collaborators Aug 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Slow loading time when listing function apps
2 participants