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

using lazy val http for async so can be overridden #279

Merged
merged 2 commits into from Sep 29, 2014

Conversation

Projects
None yet
2 participants
@rubbish
Contributor

rubbish commented Sep 29, 2014

We're overriding the dispatch.Http so that we can give it different configuration (request/connection timeouts, user agents, etc). However, we noticed that we were generating many threads from many different netty NioWorkers. Looking into it more, it turns out each AsyncHttpClientConfig.Builder spawns a thread pool of these. And looking at our heap dumps, we found that there are many AsyncHttpClientConfig.Builders within the heap.

Anyways, we found out that due to the scala trait initialization order, the scalaxb.DispatchHttpClient was always initializing a new Http even though we were overriding it. So by making it a lazy val, we can safely override it without worrying about it getting initialized.

@eed3si9n

This comment has been minimized.

Show comment
Hide comment
@eed3si9n

eed3si9n Sep 29, 2014

Owner

Could you add a comment to the effect in the code since the change is too subtle and I don't want that to be rolled back by mistake in the future. It could be something like:

// Keep it lazy. See https://github.com/eed3si9n/scalaxb/pull/279
Owner

eed3si9n commented Sep 29, 2014

Could you add a comment to the effect in the code since the change is too subtle and I don't want that to be rolled back by mistake in the future. It could be something like:

// Keep it lazy. See https://github.com/eed3si9n/scalaxb/pull/279

@eed3si9n eed3si9n added the bug label Sep 29, 2014

@rubbish

This comment has been minimized.

Show comment
Hide comment
@rubbish

rubbish Sep 29, 2014

Contributor

@eed3si9n Added.

Contributor

rubbish commented Sep 29, 2014

@eed3si9n Added.

eed3si9n added a commit that referenced this pull request Sep 29, 2014

Merge pull request #279 from Banno/http-leak-fix
using lazy val http for async so can be overridden

@eed3si9n eed3si9n merged commit abc5069 into eed3si9n:master Sep 29, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment