Skip to content

HTTPCORE-532#68

Closed
xiaohu-zhang wants to merge 1 commit intoapache:masterfrom
xiaohu-zhang:lazyinitIOReactor
Closed

HTTPCORE-532#68
xiaohu-zhang wants to merge 1 commit intoapache:masterfrom
xiaohu-zhang:lazyinitIOReactor

Conversation

@xiaohu-zhang
Copy link
Contributor

IOReactor thread can be lazy started

@xiaohu-zhang
Copy link
Contributor Author

there is somthing wrong with the code ,let me correct it first

Copy link
Member

@ok2c ok2c left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most importantly, what are the benefits of starting I/O reactor threads lazily. I see absolutely none.

}))) {
if (status.compareAndSet(Status.READY, Status.RUNNING)) {
ioReactorRef.get().start();
if(this instanceof AsyncServer){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xiaohu-zhang This is terrible. Even for test classes we should not be doing things like that.

public void start() {
ioReactor.start();
public void start(final int i) {
throw new RuntimeException("can not call start(i) method with AsyncServer class,maybe call serverStart method?");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xiaohu-zhang I cannot commit changes like that.

@xiaohu-zhang
Copy link
Contributor Author

when use few connections,for example ,use always one connection in connection pool each time(), or no http connection used when project started.framework can use less thread ,that will reduce the thread number and load in computer.

IOReactor thread can be lazy started
@ok2c
Copy link
Member

ok2c commented Jul 10, 2018

@xiaohu-zhang If you anticipate few connections and want fewer threads you really should configure fewer workers. Overall, I think this change-set causes more problems than it solves.

@xiaohu-zhang
Copy link
Contributor Author

what if can not anticipate ?lazy init is little useful in performance。Also the code can be modified do not use instanceof .after all , it depends on you.

@ok2c
Copy link
Member

ok2c commented Jul 10, 2018

@xiaohu-zhang Why? The I/O reactor distributes sessions across all available workers equally. If you have n worker threads, all of them will get started after n connections.

@xiaohu-zhang
Copy link
Contributor Author

yes.it has a little useful only when connections less than worker thread,and is not anticipate by user before.
just tiny improvement.

@ok2c
Copy link
Member

ok2c commented Jul 18, 2018

@xiaohu-zhang Please close this PR.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants