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

HTTPHeaders.description is unnecessarily slow #2930

Open
weissi opened this issue Oct 16, 2024 · 4 comments
Open

HTTPHeaders.description is unnecessarily slow #2930

weissi opened this issue Oct 16, 2024 · 4 comments

Comments

@weissi
Copy link
Member

weissi commented Oct 16, 2024

Expected behavior

HTTPHeaders.description should be reasonably fast and shouldn't do any dynamic debugPrint(...)

Actual behavior

It's slow. Look at this image: about 15% of the overall runtime of a client hitting /dynamic/info is just spent doing header printing...

Screenshot 2024-10-16 at 12 35 25 pm

That's largely because of debugPrint etc.

Printing HTTP headers is somewhat common and shouldn't be unnecessarily fast. The representation could also be nicer.

@supersonicbyte
Copy link
Contributor

I've looked at the code base a bit on HTTPHeaders and I see that the description just calls the description on the underlaying array of tuples:

public var description: String {

Maybe I am not looking at the proper type?

@weissi
Copy link
Member Author

weissi commented Oct 18, 2024

I've looked at the code base a bit on HTTPHeaders and I see that the description just calls the description on the underlaying array of tuples:

public var description: String {

Maybe I am not looking at the proper type?

You are and that's the problem. Array uses a lot of unnecessary dynamism that we don't need here because it's just strings. So if you just did something like

return self.headers.lazy.map { header in
    "\(header.name): \(header.value)"
}.joined(separator: "; ")

or something similar, then it should be much faster.

We should discuss the format we want to return here. CC @Lukasa WDYT, format for HTTP headers for description/debugDescription?

@Lukasa
Copy link
Contributor

Lukasa commented Oct 18, 2024

We can do that, though the resolution of that format is poor: it'll mishandle some things that are otherwise fine. But for a first order approximation that output is ok.

@supersonicbyte
Copy link
Contributor

@weissi thanks for the explanation, that's definitely todays TIL for me. I'm gonna be careful on calling description on array from now on!

If everyone agrees I can make a PR with the changes @weissi suggested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants