Performance of `Secure.set_headers(response)`

[davidwtbuxton]

Hi, thanks for making this project open source, it is cool.

I was looking at the implementation of Secure.set_headers(..) and it looks like the current code checks the type of the provided response for each header name/value pair that will be added to the response.

When there are many headers to add to the response, it would be more efficient to check the response type before looping through the headers, that is check the response type 1 time at the beginning of set_headers(..).

I.e instead of

for header_name, header_value in self.headers.items():
    if isinstance(response, SetHeaderProtocol):
        # If response has set_header method, use it
        set_header = response.set_header
        set_header(header_name, header_value)

something more like:

if isinstance(response, SetHeaderProtocol):
    set_header = response.set_header

for header_name, header_value in self.headers.items():
    set_header(header_name, header_value)

(Obvs an actual implementation needs to handle how different response types have different calling signatures for setting headers, and other stuff that I am ignoring that may mean my idea does not work in practice.)

I haven't tried coding this up or measuring the performance difference, but my guess is it would be much more efficient to only check the response type once for each call to Secure.set_headers(..).

Thanks,

David

[cak]

Great catch, David, and thanks for your feedback!

I appreciate you taking the time to dig into the Secure.set_headers(..) implementation. You’re absolutely right, checking the response type before looping through the headers would likely improve efficiency, especially when adding multiple headers. I’ll work on implementing a fix soon, ensuring it handles the various response types appropriately.

Thanks again for raising this and contributing to the project!

[cak]

@davidwtbuxton Thanks again for raising the issue! Could you please review the fix in 487f751 to ensure it addresses the problem?

[cak]

Thanks again @davidwtbuxton!