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

Do headers translation only once #113

Merged
merged 1 commit into from Nov 22, 2018

Conversation

Projects
None yet
2 participants
@pushkarnk
Copy link
Contributor

pushkarnk commented Nov 22, 2018

Translating HTTPHeaders to HeadersContainer is an inevitable and expensive task. Given that the HTTPHeaders from HTTPRequestHead will not change, the translation could be a one-time thing. Currently, we do the translation everytime the headers property is accessed. We could do an eager translation in the initializer or a lazy translation when the property is accessed for the first time. Given that Kitura will most likely access the headers at least once, an eager translation shouldn't be a problem here.

@pushkarnk pushkarnk requested a review from nethraravindran Nov 22, 2018

public var headers : HeadersContainer {
return HeadersContainer.create(from: _headers)
}
public var headers : HeadersContainer

private var _headers: HTTPHeaders

This comment has been minimized.

@nethraravindran

nethraravindran Nov 22, 2018

Contributor

Should we have _headers as well?

This comment has been minimized.

@nethraravindran

nethraravindran Nov 22, 2018

Contributor

The only place where we use is to get Host. Can we now re-use headers.httpHeader() to obtain the same if it is not expensive?

This comment has been minimized.

@pushkarnk

pushkarnk Nov 22, 2018

Contributor

@nethraravindran Yes, I agree. I've made the change.

Do headers translation only once
Translating HTTPHeaders to HeadersContainer is an inevitable and expensive task. Given that the HTTPHeaders from HTTPRequestHead will not change, the translation could be a one-time thing. Currently, we do the translation everytime the `headers` property is accessed. We could do an eager translation in the initializer or a lazy translation when the property is accessed for the first time. Given that Kitura will most likely access the headers at least once, an eager translation shouldn't be a problem here.

@pushkarnk pushkarnk force-pushed the pushkarnk:headers-translation branch from f648cf2 to 001b4a5 Nov 22, 2018

@pushkarnk pushkarnk merged commit c895b61 into IBM-Swift:master Nov 22, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment