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.

Sign up for GitHub

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

Jump to bottom

improve performance one step at a time #734

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

improve performance one step at a time #734

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions include/ada/url-inl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ namespace ada {
return !host.has_value() || host.value().empty() ||
type == ada::scheme::type::FILE;
}
[[nodiscard]] inline bool url::has_empty_hostname() const noexcept {
[[nodiscard]] constexpr bool url::has_empty_hostname() const noexcept {
if (!host.has_value()) {
return false;
}
return host.value().empty();
}
[[nodiscard]] inline bool url::has_hostname() const noexcept {
[[nodiscard]] constexpr bool url::has_hostname() const noexcept {
return host.has_value();
}
inline std::ostream &operator<<(std::ostream &out, const ada::url &u) {
Expand Down Expand Up @@ -188,7 +188,7 @@ inline void url::copy_scheme(const ada::url &u) {
if (has_credentials()) {
output += username;
if (!password.empty()) {
output += ":" + get_password();
output += ":" + std::string(get_password());
Copy link
Member

Choose a reason for hiding this comment

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

hmmm?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was returning string& before now string view. I'll land these changes in a different PR to improve reviews

}
output += "@";
}
Expand Down
12 changes: 6 additions & 6 deletions include/ada/url.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,11 @@ struct url : url_base {
std::optional<std::string> hash{};

/** @return true if it has an host but it is the empty string */
[[nodiscard]] inline bool has_empty_hostname() const noexcept;
[[nodiscard]] constexpr bool has_empty_hostname() const noexcept;
/** @return true if the URL has a (non default) port */
[[nodiscard]] inline bool has_port() const noexcept;
/** @return true if it has a host (included an empty host) */
[[nodiscard]] inline bool has_hostname() const noexcept;
[[nodiscard]] constexpr bool has_hostname() const noexcept;
[[nodiscard]] bool has_valid_domain() const noexcept override;

/**
Expand Down Expand Up @@ -141,15 +141,15 @@ struct url : url_base {
* @return a newly allocated string.
* @see https://url.spec.whatwg.org/#dom-url-hostname
*/
[[nodiscard]] std::string get_hostname() const noexcept;
[[nodiscard]] constexpr std::string_view get_hostname() const noexcept;

/**
* The pathname getter steps are to return the result of URL path serializing
* this's URL.
* @return a newly allocated string.
* @see https://url.spec.whatwg.org/#dom-url-pathname
*/
[[nodiscard]] std::string_view get_pathname() const noexcept;
[[nodiscard]] constexpr std::string_view get_pathname() const noexcept;

/**
* Compute the pathname length in bytes without instantiating a view or a
Expand All @@ -171,7 +171,7 @@ struct url : url_base {
* @return a constant reference to the underlying string.
* @see https://url.spec.whatwg.org/#dom-url-username
*/
[[nodiscard]] const std::string &get_username() const noexcept;
[[nodiscard]] constexpr std::string_view get_username() const noexcept;

/**
* @return Returns true on successful operation.
Expand Down Expand Up @@ -237,7 +237,7 @@ struct url : url_base {
* @return a constant reference to the underlying string.
* @see https://url.spec.whatwg.org/#dom-url-password
*/
[[nodiscard]] const std::string &get_password() const noexcept;
[[nodiscard]] constexpr std::string_view get_password() const noexcept;

/**
* Return this's URL's port, serialized.
Expand Down
51 changes: 25 additions & 26 deletions include/ada/url_aggregator-inl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,10 +451,9 @@ inline void url_aggregator::update_base_password(const std::string_view input) {
return;
}

bool password_exists = has_password();
uint32_t difference = uint32_t(input.size());
auto difference = static_cast<uint32_t>(input.size());

if (password_exists) {
if (has_password()) {
uint32_t current_length =
components.host_start - components.username_end - 1;
buffer.erase(components.username_end + 1, current_length);
Expand Down Expand Up @@ -493,7 +492,7 @@ inline void url_aggregator::append_base_password(const std::string_view input) {
ADA_ASSERT_TRUE(!helpers::overlaps(input, buffer));
#if ADA_DEVELOPMENT_CHECKS
// computing the expected password.
std::string password_expected = std::string(get_password());
auto password_expected = std::string(get_password());
password_expected.append(input);
#endif // ADA_DEVELOPMENT_CHECKS
add_authority_slashes_if_needed();
Expand All @@ -503,7 +502,7 @@ inline void url_aggregator::append_base_password(const std::string_view input) {
return;
}

uint32_t difference = uint32_t(input.size());
auto difference = static_cast<uint32_t>(input.size());
if (has_password()) {
buffer.insert(components.host_start, input);
} else {
Expand Down Expand Up @@ -548,7 +547,7 @@ inline void url_aggregator::update_base_port(uint32_t input) {
// calling std::to_string(input.value()) is unfortunate given that the port
// value is probably already available as a string.
std::string value = helpers::concat(":", std::to_string(input));
uint32_t difference = uint32_t(value.size());
auto difference = static_cast<uint32_t>(value.size());

if (components.port != url_components::omitted) {
difference -= components.pathname_start - components.host_end;
Expand All @@ -568,7 +567,7 @@ inline void url_aggregator::update_base_port(uint32_t input) {
ADA_ASSERT_TRUE(validate());
}

inline void url_aggregator::clear_port() {
constexpr void url_aggregator::clear_port() {
ada_log("url_aggregator::clear_port");
ADA_ASSERT_TRUE(validate());
if (components.port == url_components::omitted) {
Expand All @@ -587,7 +586,7 @@ inline void url_aggregator::clear_port() {
ADA_ASSERT_TRUE(validate());
}

[[nodiscard]] inline uint32_t url_aggregator::retrieve_base_port() const {
[[nodiscard]] constexpr uint32_t url_aggregator::retrieve_base_port() const {
ada_log("url_aggregator::retrieve_base_port");
return components.port;
}
Expand All @@ -610,9 +609,9 @@ inline void url_aggregator::clear_search() {
components.search_start = url_components::omitted;

#if ADA_DEVELOPMENT_CHECKS
ADA_ASSERT_EQUAL(get_search(), "",
"search should have been cleared on buffer=" + buffer +
" with " + components.to_string() + "\n" + to_diagram());
ADA_ASSERT_TRUE(get_search().empty(),
"search should have been cleared on buffer=" + buffer +
" with " + components.to_string() + "\n" + to_diagram());
#endif
ADA_ASSERT_TRUE(validate());
}
Expand All @@ -627,7 +626,7 @@ inline void url_aggregator::clear_hash() {
components.hash_start = url_components::omitted;

#if ADA_DEVELOPMENT_CHECKS
ADA_ASSERT_EQUAL(get_hash(), "",
ADA_ASSERT_TRUE(get_hash().empty(),
"hash should have been cleared on buffer=" + buffer +
" with " + components.to_string() + "\n" + to_diagram());
#endif
Expand All @@ -637,7 +636,7 @@ inline void url_aggregator::clear_hash() {
inline void url_aggregator::clear_pathname() {
ada_log("url_aggregator::clear_pathname");
ADA_ASSERT_TRUE(validate());
uint32_t ending_index = uint32_t(buffer.size());
auto ending_index = static_cast<uint32_t>(buffer.size());
if (components.search_start != url_components::omitted) {
ending_index = components.search_start;
} else if (components.hash_start != url_components::omitted) {
Expand All @@ -661,7 +660,7 @@ inline void url_aggregator::clear_pathname() {
}
ada_log("url_aggregator::clear_pathname completed, running checks...");
#if ADA_DEVELOPMENT_CHECKS
ADA_ASSERT_EQUAL(get_pathname(), "",
ADA_ASSERT_TRUE(get_pathname().empty(),
"pathname should have been cleared on buffer=" + buffer +
" with " + components.to_string() + "\n" + to_diagram());
#endif
Expand Down Expand Up @@ -706,22 +705,22 @@ inline void url_aggregator::clear_hostname() {
ADA_ASSERT_TRUE(validate());
}

[[nodiscard]] inline bool url_aggregator::has_hash() const noexcept {
[[nodiscard]] constexpr bool url_aggregator::has_hash() const noexcept {
ada_log("url_aggregator::has_hash");
return components.hash_start != url_components::omitted;
}

[[nodiscard]] inline bool url_aggregator::has_search() const noexcept {
[[nodiscard]] constexpr bool url_aggregator::has_search() const noexcept {
ada_log("url_aggregator::has_search");
return components.search_start != url_components::omitted;
}

ada_really_inline bool url_aggregator::has_credentials() const noexcept {
ada_really_inline constexpr bool url_aggregator::has_credentials() const noexcept {
ada_log("url_aggregator::has_credentials");
return has_non_empty_username() || has_non_empty_password();
}

inline bool url_aggregator::cannot_have_credentials_or_port() const {
constexpr bool url_aggregator::cannot_have_credentials_or_port() const {
ada_log("url_aggregator::cannot_have_credentials_or_port");
return type == ada::scheme::type::FILE ||
components.host_start == components.host_end;
Expand All @@ -732,7 +731,7 @@ url_aggregator::get_components() const noexcept {
return components;
}

[[nodiscard]] inline bool ada::url_aggregator::has_authority() const noexcept {
[[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.
Expand Down Expand Up @@ -767,28 +766,28 @@ inline void ada::url_aggregator::add_authority_slashes_if_needed() noexcept {
ADA_ASSERT_TRUE(validate());
}

inline void ada::url_aggregator::reserve(uint32_t capacity) {
constexpr void ada::url_aggregator::reserve(uint32_t capacity) {
buffer.reserve(capacity);
}

inline bool url_aggregator::has_non_empty_username() const noexcept {
constexpr bool url_aggregator::has_non_empty_username() const noexcept {
ada_log("url_aggregator::has_non_empty_username");
return components.protocol_end + 2 < components.username_end;
}

inline bool url_aggregator::has_non_empty_password() const noexcept {
constexpr bool url_aggregator::has_non_empty_password() const noexcept {
ada_log("url_aggregator::has_non_empty_password");
return components.host_start - components.username_end > 0;
}

inline bool url_aggregator::has_password() const noexcept {
constexpr bool url_aggregator::has_password() const noexcept {
ada_log("url_aggregator::has_password");
// This function does not care about the length of the password
return components.host_start > components.username_end &&
buffer[components.username_end] == ':';
}

inline bool url_aggregator::has_empty_hostname() const noexcept {
constexpr bool url_aggregator::has_empty_hostname() const noexcept {
if (!has_hostname()) {
return false;
}
Expand All @@ -801,11 +800,11 @@ inline bool url_aggregator::has_empty_hostname() const noexcept {
return components.username_end != components.host_start;
}

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

inline bool url_aggregator::has_port() const noexcept {
constexpr bool url_aggregator::has_port() const noexcept {
ada_log("url_aggregator::has_port");
// A URL cannot have a username/password/port if its host is null or the empty
// string, or its scheme is "file".
Expand Down
38 changes: 19 additions & 19 deletions include/ada/url_aggregator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,30 +65,30 @@ struct url_aggregator : url_base {
* @return a lightweight std::string_view.
* @see https://url.spec.whatwg.org/#dom-url-username
*/
[[nodiscard]] std::string_view get_username() const noexcept
[[nodiscard]] constexpr std::string_view get_username() const noexcept
ada_lifetime_bound;
/**
* The password getter steps are to return this's URL's password.
* This function does not allocate memory.
* @return a lightweight std::string_view.
* @see https://url.spec.whatwg.org/#dom-url-password
*/
[[nodiscard]] std::string_view get_password() const noexcept
[[nodiscard]] constexpr std::string_view get_password() const noexcept
ada_lifetime_bound;
/**
* Return this's URL's port, serialized.
* This function does not allocate memory.
* @return a lightweight std::string_view.
* @see https://url.spec.whatwg.org/#dom-url-port
*/
[[nodiscard]] std::string_view get_port() const noexcept ada_lifetime_bound;
[[nodiscard]] constexpr std::string_view get_port() const noexcept ada_lifetime_bound;
/**
* Return U+0023 (#), followed by this's URL's fragment.
* This function does not allocate memory.
* @return a lightweight std::string_view..
* @see https://url.spec.whatwg.org/#dom-url-hash
*/
[[nodiscard]] std::string_view get_hash() const noexcept ada_lifetime_bound;
[[nodiscard]] constexpr std::string_view get_hash() const noexcept ada_lifetime_bound;
/**
* Return url's host, serialized, followed by U+003A (:) and url's port,
* serialized.
Expand All @@ -97,7 +97,7 @@ struct url_aggregator : url_base {
* @return a lightweight std::string_view.
* @see https://url.spec.whatwg.org/#dom-url-host
*/
[[nodiscard]] std::string_view get_host() const noexcept ada_lifetime_bound;
[[nodiscard]] constexpr std::string_view get_host() const noexcept;
/**
* Return this's URL's host, serialized.
* This function does not allocate memory.
Expand Down Expand Up @@ -144,7 +144,7 @@ struct url_aggregator : url_base {
* A URL includes credentials if its username or password is not the empty
* string.
*/
[[nodiscard]] ada_really_inline bool has_credentials() const noexcept;
[[nodiscard]] ada_really_inline constexpr bool has_credentials() const noexcept;

/**
* Useful for implementing efficient serialization for the URL.
Expand Down Expand Up @@ -186,23 +186,23 @@ struct url_aggregator : url_base {
[[nodiscard]] bool validate() const noexcept;

/** @return true if it has an host but it is the empty string */
[[nodiscard]] inline bool has_empty_hostname() const noexcept;
[[nodiscard]] constexpr bool has_empty_hostname() const noexcept;
/** @return true if it has a host (included an empty host) */
[[nodiscard]] inline bool has_hostname() const noexcept;
[[nodiscard]] constexpr bool has_hostname() const noexcept;
/** @return true if the URL has a non-empty username */
[[nodiscard]] inline bool has_non_empty_username() const noexcept;
[[nodiscard]] constexpr bool has_non_empty_username() const noexcept;
/** @return true if the URL has a non-empty password */
[[nodiscard]] inline bool has_non_empty_password() const noexcept;
[[nodiscard]] constexpr bool has_non_empty_password() const noexcept;
/** @return true if the URL has a (non default) port */
[[nodiscard]] inline bool has_port() const noexcept;
[[nodiscard]] constexpr bool has_port() const noexcept;
/** @return true if the URL has a password */
[[nodiscard]] inline bool has_password() const noexcept;
[[nodiscard]] constexpr bool has_password() const noexcept;
/** @return true if the URL has a hash component */
[[nodiscard]] inline bool has_hash() const noexcept override;
[[nodiscard]] constexpr bool has_hash() const noexcept override;
/** @return true if the URL has a search component */
[[nodiscard]] inline bool has_search() const noexcept override;
[[nodiscard]] constexpr bool has_search() const noexcept override;

inline void clear_port();
constexpr void clear_port();
inline void clear_hash();
inline void clear_search() override;

Expand Down Expand Up @@ -233,7 +233,7 @@ struct url_aggregator : url_base {
* To optimize performance, you may indicate how much memory to allocate
* within this instance.
*/
inline void reserve(uint32_t capacity);
constexpr void reserve(uint32_t capacity);

ada_really_inline size_t parse_port(
std::string_view view, bool check_trailing_content) noexcept override;
Expand Down Expand Up @@ -268,7 +268,7 @@ struct url_aggregator : url_base {
* A URL cannot have a username/password/port if its host is null or the empty
* string, or its scheme is "file".
*/
[[nodiscard]] inline bool cannot_have_credentials_or_port() const;
[[nodiscard]] constexpr bool cannot_have_credentials_or_port() const;

template <bool override_hostname = false>
bool set_host_or_hostname(std::string_view input);
Expand All @@ -289,7 +289,7 @@ struct url_aggregator : url_base {
inline void append_base_password(std::string_view input);
inline void update_base_port(uint32_t input);
inline void append_base_pathname(std::string_view input);
[[nodiscard]] inline uint32_t retrieve_base_port() const;
[[nodiscard]] constexpr uint32_t retrieve_base_port() const;
inline void clear_hostname();
inline void clear_password();
inline void clear_pathname() override;
Expand All @@ -301,7 +301,7 @@ 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]] inline bool has_authority() const noexcept;
[[nodiscard]] constexpr bool has_authority() const noexcept;
inline void set_protocol_as_file();
inline void set_scheme(std::string_view new_scheme) noexcept;
/**
Expand Down
Loading
Loading