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

reduce has_authority check overhead #741

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 14 additions & 16 deletions include/ada/url_aggregator-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ inline void url_aggregator::update_base_authority(
if (input_starts_with_dash) {
input.remove_prefix(2);
diff += 2; // add "//"
has_authority = true;
buffer.insert(components.protocol_end, "//");
components.username_end += 2;
}
Expand Down Expand Up @@ -274,7 +275,7 @@ inline void url_aggregator::update_base_pathname(const std::string_view input) {
delete_dash_dot();
}

if (begins_with_dashdash && !has_opaque_path && !has_authority() &&
if (begins_with_dashdash && !has_opaque_path && !has_authority &&
!has_dash_dot()) {
// If url's host is null, url does not have an opaque path, url's path's
// size is greater than 1, then append U+002F (/) followed by U+002E (.) to
Expand Down Expand Up @@ -672,10 +673,9 @@ constexpr void url_aggregator::clear_pathname() {
constexpr void url_aggregator::clear_hostname() {
ada_log("url_aggregator::clear_hostname");
ADA_ASSERT_TRUE(validate());
if (!has_authority()) {
if (!has_authority) {
return;
}
ADA_ASSERT_TRUE(has_authority());

uint32_t hostname_length = components.host_end - components.host_start;
uint32_t start = components.host_start;
Expand All @@ -699,7 +699,7 @@ constexpr void url_aggregator::clear_hostname() {
"hostname should have been cleared on buffer=" + buffer +
" with " + components.to_string() + "\n" + to_diagram());
#endif
ADA_ASSERT_TRUE(has_authority());
ADA_ASSERT_TRUE(has_authority);
ADA_ASSERT_EQUAL(has_empty_hostname(), true,
"hostname should have been cleared on buffer=" + buffer +
" with " + components.to_string() + "\n" + to_diagram());
Expand Down Expand Up @@ -732,28 +732,19 @@ url_aggregator::get_components() const noexcept {
return components;
}

[[nodiscard]] constexpr bool ada::url_aggregator::has_authority()
const noexcept {
ada_log("url_aggregator::has_authority");
// Performance: instead of doing this potentially expensive check, we could
// have a boolean in the struct.
return components.protocol_end + 2 <= components.host_start &&
helpers::substring(buffer, components.protocol_end,
components.protocol_end + 2) == "//";
}

inline void ada::url_aggregator::add_authority_slashes_if_needed() noexcept {
ada_log("url_aggregator::add_authority_slashes_if_needed");
ADA_ASSERT_TRUE(validate());
// Protocol setter will insert `http:` to the URL. It is up to hostname setter
// to insert
// `//` initially to the buffer, since it depends on the hostname existence.
if (has_authority()) {
if (has_authority) {
return;
}
// Performance: the common case is components.protocol_end == buffer.size()
// Optimization opportunity: in many cases, the "//" is part of the input and
// the insert could be fused with another insert.
has_authority = true;
buffer.insert(components.protocol_end, "//");
components.username_end += 2;
components.host_start += 2;
Expand Down Expand Up @@ -803,7 +794,7 @@ constexpr bool url_aggregator::has_empty_hostname() const noexcept {
}

constexpr bool url_aggregator::has_hostname() const noexcept {
return has_authority();
return has_authority;
}

constexpr bool url_aggregator::has_port() const noexcept {
Expand Down Expand Up @@ -1087,6 +1078,13 @@ constexpr void url_aggregator::set_protocol_as_file() {
}
}

// Check for has_authority
bool expect_authority = components.host_start == components.protocol_end + 2;
if (expect_authority != has_authority) {
ada_log("has_authority value is wrong \n", to_diagram());
return false;
}

return true;
}

Expand Down
2 changes: 1 addition & 1 deletion include/ada/url_aggregator.h
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ struct url_aggregator : url_base {

std::string buffer{};
url_components components{};
bool has_authority{false};

/**
* Returns true if neither the search, nor the hash nor the pathname
Expand Down Expand Up @@ -302,7 +303,6 @@ struct url_aggregator : url_base {
std::string_view input);
ada_really_inline uint32_t replace_and_resize(uint32_t start, uint32_t end,
std::string_view input);
[[nodiscard]] constexpr bool has_authority() const noexcept;
constexpr void set_protocol_as_file();
inline void set_scheme(std::string_view new_scheme) noexcept;
/**
Expand Down
5 changes: 3 additions & 2 deletions src/url_aggregator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,8 @@ bool url_aggregator::set_pathname(const std::string_view input) {
}
clear_pathname();
parse_path(input);
if (get_pathname().starts_with("//") && !has_authority() && !has_dash_dot()) {
if (get_pathname().starts_with("//") && !has_authority && !has_dash_dot()) {
has_authority = true;
buffer.insert(components.pathname_start, "/.");
components.pathname_start += 2;
}
Expand Down Expand Up @@ -358,7 +359,7 @@ ada_really_inline void url_aggregator::parse_path(std::string_view input) {
} else {
// Non-special URLs with an empty host can have their paths erased
// Path-only URLs cannot have their paths erased
if (components.host_start == components.host_end && !has_authority()) {
if (components.host_start == components.host_end && !has_authority) {
update_base_pathname("/");
}
}
Expand Down
Loading