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

[feautre request] Add a util method to get maxDirectMemory #12103

Closed
shoothzj opened this issue Feb 16, 2022 · 7 comments · Fixed by #12118
Closed

[feautre request] Add a util method to get maxDirectMemory #12103

shoothzj opened this issue Feb 16, 2022 · 7 comments · Fixed by #12118

Comments

@shoothzj
Copy link
Contributor

Netty's PlatformDependent.maxDirectMemory() is powerful and general.
In other project, we want to read the maxDirectMemory too.
But we can't directly use PlatformDependent.maxDirectMemory() , because it will wrong if user set io.netty.maxDirectMemory greater than zero.
Instead of copy and paste netty's code(Under Apache License), I want a UtilityClass to get PlatformDependent.maxDirectMemory0 result.

I am not very familiar with netty code, I can work on this under guides.

Thanks for your help.

@normanmaurer
Copy link
Member

normanmaurer commented Feb 16, 2022

@chrisvest what do you think... Not sure we want to expose just another public method. I guess we could do it tho

@franz1981
Copy link
Contributor

@normanmaurer it's -1 to me, but I would like to understand why @shoothzj need it/the exact use case

@shoothzj
Copy link
Contributor Author

@franz1981 It's started with apache/bookkeeper#2989 when locating a Out of DirectMemory Error. We found the the maxDirectMemory read by bookkeeper is not correct with huawei jdk 11, but the result read by netty is correct.
I want to improve the maxDirectMemory readed by bookkeepe but without the io.netty.maxDirectMemory.

Please point out if I didn't make it clear. My English is not good. Thanks
cc @normanmaurer

@franz1981
Copy link
Contributor

franz1981 commented Feb 16, 2022

wdyt @normanmaurer ?

I know that Netty's OutOfDirectMemoryError throwing condition is tied with the Netty's notion of max direct memory, hence if we allow the exception to leak ie OutOfDirectMemoryError is public, maybe makes sense our limits to leak as well.
Another option for @shoothzj could be to react to such information (if structured to be used) while the limit is reached, but we need to change OutOfDirectMemoryError in order to expose exception fields that represent our limits/current values ie
requestedCapacity, usedCapacity, DIRECT_MEMORY_LIMIT as max allowed capacity

@chrisvest
Copy link
Contributor

@normanmaurer @franz1981 I don't see a big problem in exposing maxDirectMemory0 as estimateMaxDirectMemory.

@johnou
Copy link
Contributor

johnou commented Feb 18, 2022

technically it wouldn't even be part of the public API, last I recall PlatformDependent is in the "internal" package, that said could annotate it as UnstableApi?

@franz1981
Copy link
Contributor

Let's wait what @normanmaurer answer on this, but I like both @chrisvest and @johnou answers

chrisvest added a commit to chrisvest/netty that referenced this issue Feb 22, 2022
Motivation:
Integrators may want to also have a cross-platform way to compute the max direct memory.
Since we do this already, we can just as well expose the method.
However, all methods in PlatformDependent remain internal and unsupported, so callers should be robust to changing signatures and method names.

Modification:
Rename the maxDirectMemory0 method to estimateMaxDirectMemory and make it public.

Result:
Fixes netty#12103
normanmaurer pushed a commit that referenced this issue Feb 23, 2022
Motivation:
Integrators may want to also have a cross-platform way to compute the max direct memory.
Since we do this already, we can just as well expose the method.
However, all methods in PlatformDependent remain internal and unsupported, so callers should be robust to changing signatures and method names.

Modification:
Rename the maxDirectMemory0 method to estimateMaxDirectMemory and make it public.

Result:
Fixes #12103
@normanmaurer normanmaurer added this to the 4.1.75.Final milestone Feb 23, 2022
normanmaurer pushed a commit that referenced this issue Feb 23, 2022
Motivation:
Integrators may want to also have a cross-platform way to compute the max direct memory.
Since we do this already, we can just as well expose the method.
However, all methods in PlatformDependent remain internal and unsupported, so callers should be robust to changing signatures and method names.

Modification:
Rename the maxDirectMemory0 method to estimateMaxDirectMemory and make it public.

Result:
Fixes #12103
raidyue pushed a commit to raidyue/netty that referenced this issue Jul 8, 2022
Motivation:
Integrators may want to also have a cross-platform way to compute the max direct memory.
Since we do this already, we can just as well expose the method.
However, all methods in PlatformDependent remain internal and unsupported, so callers should be robust to changing signatures and method names.

Modification:
Rename the maxDirectMemory0 method to estimateMaxDirectMemory and make it public.

Result:
Fixes netty#12103
franz1981 pushed a commit to franz1981/netty that referenced this issue Aug 22, 2022
Motivation:
Integrators may want to also have a cross-platform way to compute the max direct memory.
Since we do this already, we can just as well expose the method.
However, all methods in PlatformDependent remain internal and unsupported, so callers should be robust to changing signatures and method names.

Modification:
Rename the maxDirectMemory0 method to estimateMaxDirectMemory and make it public.

Result:
Fixes netty#12103
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants