refactor(http): Use query string helper

[suneettipirneni]

This is created based on the discussion here as well as here.

[laralove143]

thank you for the great work!

[vilgotf]

We've been very strict about Display performance so this is an unacceptable regression. Since query_string_builder cannot be made lazy (a requirement for us) I propose vendoring a similar API, without allocations. We can iterate on such an API on Discord

[sunsided]

We've been very strict about Display performance so this is an unacceptable regression. Since query_string_builder cannot be made lazy (a requirement for us) I propose vendoring a similar API, without allocations. We can iterate on such an API on Discord

Author of the library here. I'd love to understand the use case. Is this simply about deferring the call to to_string() until the query string is actually to be rendered?

[laralove143]

We've been very strict about Display performance so this is an unacceptable regression. Since query_string_builder cannot be made lazy (a requirement for us) I propose vendoring a similar API, without allocations. We can iterate on such an API on Discord

while i agree with the (arguably little) performance penalty of using this method, i think we need to consider the 20/80 rule here

while a simple change from format!() to f.write_str() is worth it for the performance gain, writing our own query string builder might not be worth it

not to mention, i feel that this goes beyond the scope of twilight, if we can find a library that fits our criteria better or if someone volunteers to write such a library, i agree that we should use that library instead

[vilgotf]

Author of the library here. I'd love to understand the use case. Is this simply about deferring the call to to_string() until the query string is actually to be rendered?

My usage of "lazy" was not quite correct. What I meant is that we want to avoid creating temporary Strings for keys and values. We have a very big performance focus so I'm unsure if whatever API we come up with here should be broadly encouraged.

[sunsided]

Author of the library here. I'd love to understand the use case. Is this simply about deferring the call to to_string() until the query string is actually to be rendered?

My usage of "lazy" was not quite correct. What I meant is that we want to avoid creating temporary Strings for keys and values. We have a very big performance focus so I'm unsure if whatever API we come up with here should be broadly encouraged.

Thanks for clarifying! I was experimenting with some deferred rendering versions of the builder but it inevitably ends up requiring lifetime constrains. It appears that in the way the builder is used now this could work out trivially, but I'll have to wrap my head around how to do it best. The current version of the library already has some optimizations, but it's far from trivial: The percent_encoding library itself requires a &str or &[u8], so right now, at least one throwaway string representation must be generated either way.

[vilgotf]

Looks good, just some code style questions.

Two Approvals