Skip to content

perf improvement: remove hyper_client::host_port_from_uri#318

Open
ZhidongPeng wants to merge 1 commit intoAzure:devfrom
ZhidongPeng:host
Open

perf improvement: remove hyper_client::host_port_from_uri#318
ZhidongPeng wants to merge 1 commit intoAzure:devfrom
ZhidongPeng:host

Conversation

@ZhidongPeng
Copy link
Collaborator

Parse Host & Port from UrI multiple times per each proxied requests - Removed host_port_from_uri(full_url: &Uri) and added struct HostEndpoint with host, port and path_and_query

@ZhidongPeng ZhidongPeng changed the title remove hyper_client::host_port_from_uri to avoid parse Host & Port from UrI multiple times perf improvement: remove hyper_client::host_port_from_uri Feb 10, 2026
Ipv4Addr::LOCALHOST.to_string(),
self.port,
PROVISION_URL_PATH
PROVISION_URL_PATH,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit

(format!("http://{}/", constants::WIRE_SERVER_IP))
.parse()
.unwrap(),
constants::WIRE_SERVER_IP.to_string(),
Copy link
Contributor

@shahneerali shahneerali Feb 13, 2026

Choose a reason for hiding this comment

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

what is 80 in this case?

}

const IMDS_URI: &str = "metadata/instance?api-version=2018-02-01";
const IMDS_URI: &str = "/metadata/instance?api-version=2018-02-01";
Copy link
Contributor

Choose a reason for hiding this comment

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

need to validate this with UT


const TELEMETRY_DATA_URI: &str = "machine/?comp=telemetrydata";
const GOALSTATE_URI: &str = "machine?comp=goalstate";
const TELEMETRY_DATA_URI: &str = "/machine/?comp=telemetrydata";
Copy link
Contributor

Choose a reason for hiding this comment

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

again need tests for this change

}
};
let default_port = if uri.scheme_str() == Some("https") {
443
Copy link
Contributor

Choose a reason for hiding this comment

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

let's make constants for these

let default_port = if uri.scheme_str() == Some("https") {
443
} else {
80
Copy link
Contributor

Choose a reason for hiding this comment

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

same as prev comment

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