use netty PlatformDependent.estimateMaxDirectMemory

[hezhangjian]

Motivation

• PlatformDependent.maxDirectMemory() can be inaccurate if io.netty.maxDirectMemory are setted
• Bookkeeper's DirectMemoryUtils is not worked within some jdk releases.
  In netty 4.1.75, they introduced a new method PlatformDependent.estimateMaxDirectMemory to help users get maxDirectMemory. Since netty's this method works well on many jdk releases, use this to replace below two.
  PS: DirectMemoryUtils has been removed from bookkeeper in Use netty maxDirectMemory instead of DirectMemoryUtils bookkeeper#2989

Modifications

• use PlatformDependent.estimateMaxDirectMemory instead

[HQebupt]

LGTM :)

[hezhangjian]

/pulsarbot run-failure-checks

[michaeljmarshall]

@shoothzj - I am not familiar with direct memory computation, but given the javadoc that I share in a comment, I'd like to make sure that we don't introduce a performance regression with this PR. Let me know what you think, thanks!

[michaeljmarshall]

It looks like the javadoc for this method recommends using #maxDirectMemory():

    /**
     * Compute an estimate of the maximum amount of direct memory available to this JVM.
     * <p>
     * The computation is not cached, so you probably want to use {@link #maxDirectMemory()} instead.
     * <p>
     * This will produce debug log output when called.
     *
     * @return The estimated max direct memory, in bytes.
     */
    public static long estimateMaxDirectMemory()

https://github.com/netty/netty/blob/0d653f08cd108a3ba6cf76d9fc052b8141ea3196/common/src/main/java/io/netty/util/internal/PlatformDependent.java#L1154-L1163

I am concerned that the cost of this alternative method. Are you able to provide documentation on how this method is better? Alternatively, is there a reason we can't cache the result in this class and the other references that will be called many times?

[hezhangjian]

@michaeljmarshall My bad
The main reason I want to replace is that maxDirectMemory() can be changed by system property io.netty.maxDirectMemory, which will may lead to wrong result.
To avoid performance regression, I think we can put it in a common class, and others use the result.
What do you think?

[michaeljmarshall]

@michaeljmarshall My bad The main reason I want to replace is that maxDirectMemory() can be changed by system property io.netty.maxDirectMemory, which will may lead to wrong result. To avoid performance regression, I think we can put it in a common class, and others use the result. What do you think?

Is it possible that it is a feature, not a bug, that we expose io.netty.maxDirectMemory to end users? If a user configures io.netty.maxDirectMemory poorly, I wouldn't consider that a performance regression.

[hezhangjian]

@michaeljmarshall There are two scenes using this method

• metrics, using this method to produce maxDirectMemory metrics, estimateMaxDirectMemory is more proper.
• Serverside config maxMessagePublishBufferSizeInMB. This param's default value is 1/2 of direct memory. io.netty.maxDirectMemory isn't on our docs. They can reconfigure it by maxMessagePublishBufferSizeInMB.

[hezhangjian]

@lhotari PTAL again. @codelipenghui @eolivelli @merlimat PTAL

[coderzc]

LGTM

[lhotari]

Is it intentional to keep on using PlatformDependent class here?

[hezhangjian]

@lhotari Sorry. It's a miss. It have been fixed.

[michaeljmarshall]

LGTM. Thanks for answering my questions @shoothzj.

[eolivelli]

LGTM

[hezhangjian]

/pulsarbot rerun-failure-checks