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

Lazily initalise the HttpClient for the Bitbucket OAuth2ClientRegistry #1239

Merged
merged 2 commits into from
May 3, 2023

Conversation

mjcheetham
Copy link
Collaborator

@mjcheetham mjcheetham commented May 3, 2023

Lazily acquire the HttpClient instance from the factory inside of the OAuth2ClientRegistry. There is a relatively large cost to creating a new HttpClient instance (especially the first; due to config reading) that is being paid each time we just register the entire Bitbucket host provider.

Whilst we are here also cleanup some of the styling/field naming in this class to be consistent with the rest of GCM.

This saves approximately 120ms of startup time.

Lazily acquire the HttpClient instance from the factory inside of the
OAuth2ClientRegistry. There is a relatively large cost to creating a
new HttpClient instance (especially the first; due to config reading)
that is being paid each time we just register the entire Bitbucket host
provider.

Whilst we are here also cleanup some of the styling/field naming in
this class to be consistent with the rest of GCM.
@mjcheetham mjcheetham added host:bitbucket Specific to the Bitbucket host provider performance An issue with performance labels May 3, 2023
@mjcheetham mjcheetham requested review from mminns, vdye and ldennington May 3, 2023 18:40
Copy link
Contributor

@vdye vdye left a comment

Choose a reason for hiding this comment

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

Looks good! I had one question about ReleaseManagedResources, hopefully easy to fix or explain either way. 🚀

src/shared/Atlassian.Bitbucket/OAuth2ClientRegistry.cs Outdated Show resolved Hide resolved
@mjcheetham mjcheetham merged commit 73ddb8e into git-ecosystem:main May 3, 2023
@mjcheetham mjcheetham deleted the perf branch May 3, 2023 23:08
@mminns
Copy link
Contributor

mminns commented May 4, 2023

👍

Copy link
Contributor

@mminns mminns left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks.

mjcheetham added a commit that referenced this pull request May 8, 2023
**Changes since 2.1.0:**

- Fix several UI bugs (#1238, #1241)
- Lazily initialise Bitbucket host provider dependencies (#1239)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
host:bitbucket Specific to the Bitbucket host provider performance An issue with performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants