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.

Sign up for GitHub

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

Jump to bottom

Add the ability to configure Netty's default allocator #6661

Merged
merged 1 commit into from
Jan 20, 2022
Merged

Add the ability to configure Netty's default allocator #6661

merged 1 commit into from
Jan 20, 2022

Conversation

graemerocher
Copy link
Contributor

@graemerocher graemerocher commented Dec 15, 2021 •
edited
Loading

With the following configuration and this PR merged:

netty:
  default:
    allocator:
      max-order: 1

Memory consumption stays stable at around 24mb RSS after 5 requests. I conducted some brief benchmarking with wrk and it seems to make no difference performance wise for simple JSON messages.

We can perhaps consider making it a default for new apps in Launch that use Netty

Fixes #6623

@graemerocher graemerocher added this to the 3.3.0 milestone Dec 15, 2021
@graemerocher graemerocher added the type: enhancement New feature or request label Dec 15, 2021
@jameskleeh
Copy link
Contributor

What is the benefit for a public interface for this? Could users inject this and change the configuration post-startup and the configuration would take effect?

@graemerocher
Copy link
Contributor Author

They could use a BeanCreatedEventListener yes

@ilopmar
Copy link
Contributor

ilopmar commented Dec 16, 2021

@graemerocher just to make sure: I just need this PR and the configuration in my app, right? It is not necessary to start the native image with any -Xmx setting. It this correct?
I'm asking because I've tried this locally and the memory consumption keeps increasing for every curl request.

@graemerocher
Copy link
Contributor Author

It will go up just in smaller increments instead of increments of 16mb

@ilopmar
Copy link
Contributor

ilopmar commented Dec 16, 2021

I see the same behaviour as before.

With the changes in this PR:

USER PID VSZ RSS
ivan 61216 1146512 47M
ivan 61216 1224340 55M
ivan 61216 1301144 58M
ivan 61216 1377948 61M
ivan 61216 1454752 64M
ivan 61216 1531556 66M
ivan 61216 1608360 69M
ivan 61216 1685164 72M
ivan 61216 1761968 75M
ivan 61216 1838772 78M
ivan 61216 1915576 81M
ivan 61216 1992380 83M
ivan 61216 2069184 86M
ivan 61216 2145988 89M
ivan 61216 2222792 92M

With the latest snapshot available on OSS Sonatype:

USER PID VSZ 0M
ivan 66267 1147516 48M
ivan 66267 1241732 56M
ivan 66267 1334924 59M
ivan 66267 1428116 62M
ivan 66267 1521308 65M
ivan 66267 1614500 67M
ivan 66267 1707692 70M
ivan 66267 1800884 73M
ivan 66267 1894076 76M
ivan 66267 1987268 79M
ivan 66267 2080460 82M
ivan 66267 2173652 84M
ivan 66267 2266844 87M

@graemerocher
Copy link
Contributor Author

What I see is:

  • After startup: 13251 38247880 22M
  • After first request: graemerocher 13251 38256596 28M
  • After 5 requests: 13251 38256596 28M

So no idea what is going with your machine, perhaps someone else can try it out.

@graemerocher
Copy link
Contributor Author

This is with a simple hello world app not the sample from Jason's original issue

@ilopmar
Copy link
Contributor

ilopmar commented Dec 16, 2021

I was using our basic-app just sending a hello world request.

@graemerocher graemerocher mentioned this pull request Dec 17, 2021
Merged
@graemerocher
Copy link
Contributor Author

🤷‍♂️

@melix
Copy link
Contributor

melix commented Dec 21, 2021 •
edited
Loading

So I have tried this branch locally, and I added:

netty:
  default:
    allocator:
      max-order: 1

to my configuration. I can see an initial memory consumption of 46MB. After 5 requests, it uses 57MB and consumption is stable.

cchampeau@cchampeau-XPS-15-9570:~/DEV/PROJECTS/GITHUB/micronaut/demo-app$ ps eo command,user,pid,vsz,rss `pgrep -f 'nativeCompile/demo-app'`
COMMAND                     USER         PID    VSZ   RSS
build/native/nativeCompile/ cchampe+  174451 621740 46372
cchampeau@cchampeau-XPS-15-9570:~/DEV/PROJECTS/GITHUB/micronaut/demo-app$ ps eo command,user,pid,vsz,rss `pgrep -f 'nativeCompile/demo-app'`
COMMAND                     USER         PID    VSZ   RSS
build/native/nativeCompile/ cchampe+  174451 634032 53768
cchampeau@cchampeau-XPS-15-9570:~/DEV/PROJECTS/GITHUB/micronaut/demo-app$ ps eo command,user,pid,vsz,rss `pgrep -f 'nativeCompile/demo-app'`
COMMAND                     USER         PID    VSZ   RSS
build/native/nativeCompile/ cchampe+  174451 634032 53768
cchampeau@cchampeau-XPS-15-9570:~/DEV/PROJECTS/GITHUB/micronaut/demo-app$ ps eo command,user,pid,vsz,rss `pgrep -f 'nativeCompile/demo-app'`
COMMAND                     USER         PID    VSZ   RSS
build/native/nativeCompile/ cchampe+  174451 634032 53768

after 7 requests:

cchampeau@cchampeau-XPS-15-9570:~/DEV/PROJECTS/GITHUB/micronaut/demo-app$ ps eo command,user,pid,vsz,rss `pgrep -f 'nativeCompile/demo-app'`
COMMAND                     USER         PID    VSZ   RSS
build/native/nativeCompile/ cchampe+  174451 634032 56224

@graemerocher
Copy link
Contributor Author

@jameskleeh can this be merged?

jameskleeh
jameskleeh reviewed Jan 7, 2022
View reviewed changes
buffer-netty/src/main/java/io/micronaut/buffer/netty/ByteBufAllocatorConfiguration.java
* @author graemerocher
*/
public interface ByteBufAllocatorConfiguration {
String DEFAULT_ALLOCATOR = "netty.default.allocator";
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the value of default in the key? Are there other allocators that are not default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Potentially you could have different allocators although we don't currently allow anything but the default. Still I think it helps clarify that what you are configuring is the default allocator used by Netty

@yawkat
Copy link
Member

yawkat commented Jan 7, 2022

we still haven't decided on the netty module structure either: #6657 (comment)

@graemerocher
Copy link
Contributor Author

@yawkat it seems overkill to me to create a whole new module for just this functionality when it relates to configuring the Netty buffer which we already have an appropriately named module for.

@yawkat
Copy link
Member

yawkat commented Jan 7, 2022

though looking at this pr, it wouldnt be proper in a netty-graal module either, since it has nothing to do with graal. and three modules would be a bit much. so i change my vote to putting all the netty stuff into netty-buffer, and making gcp depend on netty-buffer :)

@jameskleeh jameskleeh merged commit d79095f into 3.3.x Jan 20, 2022
@jameskleeh jameskleeh deleted the bytebuf-allocator-config branch January 20, 2022 16:03
yawkat pushed a commit that referenced this pull request Jan 21, 2022
@graemerocher @yawkat
Add the ability to configure Netty's default allocator (#6661)
1d2a73f
@MaximeFrancoeur
Copy link

MaximeFrancoeur commented Feb 9, 2022 •
edited
Loading

@graemerocher just to make sure: I just need this PR and the configuration in my app, right? It is not necessary to start the native image with any -Xmx setting. It this correct? I'm asking because I've tried this locally and the memory consumption keeps increasing for every curl request.

Any news about this problem?
After upgraded my application with new micronaut 3.3.1 I can see a huge memory increase after millions request and run out of memory.

memory

Application running on Kubernetes cluster

this was run without ->
netty: default: allocator: max-order: 6

@graemerocher
Copy link
Contributor Author

Seems unrelated. File a ticket with steps to reproduce

@MaximeFrancoeur MaximeFrancoeur mentioned this pull request Feb 9, 2022
Closed
yawkat pushed a commit that referenced this pull request Jun 24, 2022
@graemerocher @yawkat
Add the ability to configure Netty's default allocator (#6661)
a36fc12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jameskleeh jameskleeh jameskleeh left review comments

@yawkat yawkat yawkat approved these changes

@ilopmar ilopmar Awaiting requested review from ilopmar

Assignees
No one assigned
Labels
type: enhancement New feature or request
Projects
None yet
Milestone
3.3.0
Development

Successfully merging this pull request may close these issues.

Memory Consumption Increase For Each Request
6 participants
@graemerocher @jameskleeh @ilopmar @melix @yawkat @MaximeFrancoeur