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

[#1935] Don't create CPU*2 "New I/O worker" threads for unused ports #1327

Merged
merged 1 commit into from
Mar 25, 2020

Conversation

davidcostanzo
Copy link
Contributor

This change avoids creating ServerBootstrap objects which are never bound to a port (for example, not creating the HTTPS server if the application doesn't have a https.port configured).
The change is simple, it only moves the code which instantiates these objects into the following if statement. Netty creates 2*(number of processor) "New I/O worker" threads when a NioServerSocketChannelFactory object is created, so this change avoids creating lots of threads that the application never uses.

For example, on a machine with 16 CPUs, this eliminates the creation of 32 threads that would otherwise exist for the lifetime of the Play server and never execute. The benefit for eliminating these threads:

  • Less thread pollution when viewing a Play! JVM with tools like jstack or visualvm.
  • Less memory use.
  • For organizations which impose a process limit on GNU/Linux systems, these threads counted against the user's process quota. These excess threads prevented some developers from running applications on some machines.

Background Context

The Play! code has been like this since at least 1.2.7, but the excess thread creation didn't exist until 1.3.0 when an update to the netty library changed the location from which Netty creates the threads.

In my organization, we run dozens of Play! applications on a machine with many CPUs and each of these Play! applications only has http.port configured (no https.port). End users access these Play! servers through a reverse proxy that provides the HTTPS connection. For us, the reduction in thread usage is noticeable.

This change avoids creating ServerBootstrap objects which are never
bound to a port (for example, not to create the HTTPS server if the
application doesn't configure https.port).  Netty creates
2*(number of processor) "New I/O worker" threads when a
NioServerSocketChannelFactory object is created, so this change
avoids creating lots of threads that the application never uses.
Copy link
Contributor

@asolntsev asolntsev left a comment

Choose a reason for hiding this comment

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

@xael-fry Recommended to merge.

@xael-fry xael-fry added this to the 1.5.4 milestone Mar 25, 2020
@xael-fry xael-fry merged commit e7b25e0 into playframework:master Mar 25, 2020
@xael-fry
Copy link
Member

Merged in master
Tahnks @asolntsev @davidcostanzo

@xael-fry xael-fry modified the milestone: 1.6.0 Dec 14, 2020
@davidcostanzo davidcostanzo deleted the lighthouse-1935-patch branch November 18, 2022 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants