Skip to content

Some perf improvements for uri related code #149

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

Merged
merged 3 commits into from
Jun 7, 2016
Merged

Some perf improvements for uri related code #149

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5df687f
some perf improvements for uri related code
hanzhumsft May 4, 2016
5c95411
Merge perf improvements for uri related code into uri_perf_improvements
hanzhumsft May 10, 2016
8e09ae6
fixed 2 unit tests
hanzhumsft May 18, 2016
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
10 changes: 10 additions & 0 deletions Release/include/cpprest/asyncrt_utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,11 @@ namespace conversions
return print_string(val, std::locale());
}

inline utility::string_t print_string(const utility::string_t &val)
{
return val;
}

template <typename Target>
Target scan_string(const utility::string_t &str, const std::locale &loc)
{
Expand All @@ -216,6 +221,11 @@ namespace conversions
{
return scan_string<Target>(str, std::locale());
}

inline utility::string_t scan_string(const utility::string_t &str)
{
return str;
}
}

namespace details
Expand Down
6 changes: 6 additions & 0 deletions Release/include/cpprest/base_uri.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,12 @@ namespace web {
/// </summary>
uri() { m_uri = _XPLATSTR("/");};

/// <summary>
/// Creates a URI from the given URI components.
/// </summary>
/// <param name="components">A URI components object to create the URI instance.</param>
_ASYNCRTIMP uri(const details::uri_components &components) : m_components(components) { m_uri = m_components.join(); }
Copy link
Contributor

@ras0219-msft ras0219-msft May 12, 2016

Choose a reason for hiding this comment

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

If the function is defined inline, it shouldn't be marked _ASYNCRTIMP. This can be fixed when merging.

Copy link
Member Author

@hanzhumsft hanzhumsft May 13, 2016

Choose a reason for hiding this comment

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

Yes. You are right.


/// <summary>
/// Creates a URI from the given encoded string. This will throw an exception if the string
/// does not contain a valid URI. Use uri::validate if processing user-input.
Expand Down
2 changes: 1 addition & 1 deletion Release/include/cpprest/http_headers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ class http_headers
{
if (has(name))
{
m_headers[name] = m_headers[name].append(_XPLATSTR(", ") + utility::conversions::print_string(value));
m_headers[name] = m_headers[name].append(_XPLATSTR(", ")).append(utility::conversions::print_string(value));
Copy link
Contributor

@ras0219-msft ras0219-msft May 12, 2016

Choose a reason for hiding this comment

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

Since string::append() modifies the string, we no longer need an assignment back into the map here. This can be fixed when merging.

Copy link
Member Author

@hanzhumsft hanzhumsft May 13, 2016

Choose a reason for hiding this comment

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

Right, we can just remove the assignment.

}
else
{
Expand Down
2 changes: 1 addition & 1 deletion Release/include/cpprest/uri_builder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ namespace web
uri_builder &append_query(const utility::string_t &name, const T &value, bool do_encoding = true)
{
auto encodedName = name;
auto encodedValue = ::utility::conversions::print_string(value, std::locale::classic());
auto encodedValue = ::utility::conversions::print_string(value);

if (do_encoding)
{
Expand Down
24 changes: 12 additions & 12 deletions Release/src/uri/uri.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,28 +53,27 @@ utility::string_t uri_components::join()
m_path.insert(m_path.begin(), 1, _XPLATSTR('/'));
}

utility::ostringstream_t os;
os.imbue(std::locale::classic());
utility::string_t ret;

if (!m_scheme.empty())
{
os << m_scheme << _XPLATSTR(':');
ret.append(m_scheme).append({ _XPLATSTR(':') });
}

if (!m_host.empty())
{
os << _XPLATSTR("//");
ret.append(_XPLATSTR("//"));

if (!m_user_info.empty())
{
os << m_user_info << _XPLATSTR('@');
ret.append(m_user_info).append({ _XPLATSTR('@') });
}

os << m_host;
ret.append(m_host);

if (m_port > 0)
{
os << _XPLATSTR(':') << m_port;
ret.append({ _XPLATSTR(':') }).append(utility::conversions::print_string(m_port));
}
}

Expand All @@ -83,22 +82,23 @@ utility::string_t uri_components::join()
// only add the leading slash when the host is present
if (!m_host.empty() && m_path.front() != _XPLATSTR('/'))
{
os << _XPLATSTR('/');
ret.append({ _XPLATSTR('/') });
}
os << m_path;

ret.append(m_path);
}

if (!m_query.empty())
{
os << _XPLATSTR('?') << m_query;
ret.append({ _XPLATSTR('?') }).append(m_query);
}

if (!m_fragment.empty())
{
os << _XPLATSTR('#') << m_fragment;
ret.append({ _XPLATSTR('#') }).append(m_fragment);
}

return os.str();
return ret;
}
}

Expand Down
2 changes: 1 addition & 1 deletion Release/src/uri/uri_builder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ utility::string_t uri_builder::to_string()

uri uri_builder::to_uri()
{
return uri(m_uri.join());
return uri(m_uri);
}

bool uri_builder::is_valid()
Expand Down
4 changes: 2 additions & 2 deletions Release/src/uri/uri_parser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ bool parse(const utility::string_t &encoded_string, uri_components &components)

// convert scheme to lowercase
std::transform(components.m_scheme.begin(), components.m_scheme.end(), components.m_scheme.begin(), [](utility::char_t c) {
return std::tolower(c, std::locale::classic());
return (utility::char_t)tolower(c);
});
}
else
Expand All @@ -119,7 +119,7 @@ bool parse(const utility::string_t &encoded_string, uri_components &components)

// convert host to lowercase
std::transform(components.m_host.begin(), components.m_host.end(), components.m_host.begin(), [](utility::char_t c) {
return std::tolower(c, std::locale::classic());
return (utility::char_t)tolower(c);
});
}
else
Expand Down