Closed suneettipirneni closed 2 months ago
We've been very strict about
Display
performance so this is an unacceptable regression. Sincequery_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?
We've been very strict about
Display
performance so this is an unacceptable regression. Sincequery_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
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 String
s 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.
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
String
s 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.
This is created based on the discussion here as well as here.