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

Improve performance of ReactiveAdapterRegistry [SPR-15747] #20303

Closed
spring-projects-issues opened this issue Jul 7, 2017 · 5 comments
Closed
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

Phil Webb opened SPR-15747 and commented

Whilst profiling startup times for Spring Boot I noticed that ReactiveAdapterRegistry is taking quite some time for a simple application.

For example, if I run ConfigurationClassProcessingTests.simplestPossibleConfig I get a test run time around 0.66 seconds. If I comment out the code from the ReactiveAdapterRegistry constructor it drops to 0.53 seconds.

Perhaps we can make the registry late binding, or unwind some of the existing lambda calls?


Affects: 5.0 RC2

Referenced from: commits dfcc9af

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

I can't reproduce this reliably as described.

The following gives me the same results (between 1200-1350, sometimes down to 1100) whether the ReactiveAdapterRegistry constructor is commented out or not:

@Test
public void simplestPossibleConfig() {
     StopWatch watch = new StopWatch();
     watch.start();
     BeanFactory factory = initBeanFactory(SimplestPossibleConfig.class);
     String stringBean = factory.getBean("stringBean", String.class);
     watch.stop();
     System.out.println(watch.prettyPrint());
}

Narrowing down to just the constructor, I get about 100 milliseconds in either scenario:

@Test
public void simplestPossibleConfig() {
     StopWatch watch = new StopWatch();
     watch.start();
     new ReactiveAdapterRegistry();
     watch.stop();
     System.out.println(watch.prettyPrint());
}

I'm pretty sure this all comes from the classpath detection in the static variables. The following also gives 100 milliseconds:

@Test
public void simplestPossibleConfig() {
     StopWatch watch = new StopWatch();
     watch.start();
     boolean reactorPresent = ClassUtils.isPresent("reactor.core.publisher.Flux", ReactiveAdapterRegistry.class.getClassLoader());
     boolean rxJava1Present = ClassUtils.isPresent("rx.Observable", ReactiveAdapterRegistry.class.getClassLoader());
     boolean rxReactiveStreamsPresent = ClassUtils.isPresent("rx.RxReactiveStreams", ReactiveAdapterRegistry.class.getClassLoader());
     boolean rxJava2Present = ClassUtils.isPresent("io.reactivex.Flowable", ReactiveAdapterRegistry.class.getClassLoader());
     watch.stop();
     System.out.println(watch.prettyPrint());
}

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

I've removed the static classpath checks in favor of lazily trying to register + ignore failures. Can you confirm if you see a difference?

@spring-projects-issues
Copy link
Collaborator Author

Ivan Pavlukhin commented

Hi, I am observing performance penalty on the path:

registerAdapter, ReactiveAdapterRegistry$ReactorJdkFlowAdapterRegistrar {org.springframework.core}
<init>, ReactiveAdapterRegistry {org.springframework.core}
<init>, InvocableHandlerMethod {org.springframework.web.reactive.result.method}
getRequestMappingMethod, ControllerMethodResolver {org.springframework.web.reactive.result.method.annotation}
handle, RequestMappingHandlerAdapter {org.springframework.web.reactive.result.method.annotation}

It is called for each incoming request and I observe a penalty around 2 ms for each request. Most likely it is caused by used reflection. Is there a need to create ReactiveAdapterRegistry for each request?

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

What you're describing used to be an issue that was fixed in 5.0.2. Based onthe stack trace I wouldn't expect this to be an issue currently so I'm guessing you're on 5.0.1. In any this is an old ticket that will not re-open, so please create a new ticket if necessary.

@spring-projects-issues
Copy link
Collaborator Author

Ivan Pavlukhin commented

Ah, indeed I used 5.0.1, sorry for that.

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

No branches or pull requests

2 participants