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

Too much Bootstrap and Tcp configuration related objects created for each request #991

Closed
anilgursel opened this issue Feb 6, 2020 · 4 comments
Labels
type/bug A general bug
Milestone

Comments

@anilgursel
Copy link
Contributor

The configure() method gets called from in TcpClient#connect with every single request. Calling configure() causes too much of BootStrap cloning also other setup to be done again and again. When connection pool is in use, is there a specific reason to keep running configuration with each request? For instance, for the below code example with max 10 connections in pool, I see TcpUtils.updatePort keeps getting called with every request:

import io.netty.handler.codec.http.HttpMethod;
import reactor.core.publisher.Flux;
import reactor.netty.http.HttpProtocol;
import reactor.netty.http.client.HttpClient;
import reactor.netty.resources.ConnectionProvider;

public class Sample {

    public static void main(final String[] args) throws InterruptedException {
        System.out.println("Start...");

        final HttpClient httpClient =
                HttpClient.create(ConnectionProvider.fixed("dummy", 10, 100))
                        .baseUrl("http://localhost:8080")
                        .protocol(HttpProtocol.HTTP11)
                        .keepAlive(true);

        Flux.range(1, 1000000)
                .flatMap(i ->
                        httpClient.request(HttpMethod.GET)
                                .uri("/hello")
                                .responseSingle((resp, bytes) -> bytes.asString())
                                .doOnNext(s -> System.out.println(s)), 10)
                .blockLast();

        System.out.println("End..");
    }
}

And here is the mock service:

import com.github.tomakehurst.wiremock.WireMockServer;
import com.github.tomakehurst.wiremock.core.WireMockConfiguration;

import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
import static com.github.tomakehurst.wiremock.client.WireMock.get;
import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo;
import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;

public class MockServer {


    public static void main(final String[] args) {
        final WireMockConfiguration config =
                wireMockConfig()
                        .port(8080)
                        .containerThreads(10)
                        .jettyAcceptQueueSize(10);

        final WireMockServer wireMockServer = new WireMockServer(config);
        wireMockServer.start();

        wireMockServer.stubFor(get(urlPathEqualTo("/hello"))
                .willReturn(aResponse()
                        .withBody("Hello World!")
                        .withStatus(200)));
    }
}

Please see the JFR snapshot. This is just one code path. There is quite bit of garbage created with AbstractBootstrap.copiedMap and more, because configure keeps getting called in connect():

image

image

If you can provide details about reasoning, I will very much appreciate. If not done on purpose, I will be happy to contribute if you can provide some suggestions.

  • Reactor version(s) used: 0.9.2-RELEASE
@anilgursel anilgursel added status/need-triage A new issue that still need to be evaluated as a whole type/bug A general bug labels Feb 6, 2020
@violetagg violetagg removed the status/need-triage A new issue that still need to be evaluated as a whole label Feb 12, 2020
@violetagg violetagg added this to the 1.0.0.M1 milestone Feb 12, 2020
@violetagg
Copy link
Member

violetagg commented Feb 12, 2020

@anilgursel We know about this issue and we are working on this (currently as part of 1.0.0.M1 backlog) #568

@anilgursel
Copy link
Contributor Author

@violetagg any estimation when 1.0.0.M1 would be available?

@violetagg
Copy link
Member

@anilgursel May be end of May, there is no concrete date at the moment.

@violetagg
Copy link
Member

This is addressed with #1046

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

No branches or pull requests

2 participants