Repository: netty/netty
Issue: ByteBufUtil.prettyHexDump may have a problem when calculating the initial capacity of StringBuilder

i use netty4.1.34

source code in netty4.1.34

```
private static String prettyHexDump(ByteBuf buffer, int offset, int length) {
            if (length == 0) {
              return StringUtil.EMPTY_STRING;
            } else {
                int rows = length / 16 + (length % 15 == 0? 0 : 1) + 4;
                StringBuilder buf = new StringBuilder(rows * 80);
                appendPrettyHexDump(buf, buffer, offset, length);
                return buf.toString();
            }
        }
```

in my opinion, rows = length / 16 + (length & 15 == 0? 0 : 1) + 4;

if  int rows = length / 16 + (length % 15 == 0? 0 : 1) + 4;
when length = 0 or length = 15, length % 15 always 0，this is wrong.


Issue comments:
This should be an issue. The way to find the remainder is either `length % 16` or `length & 15`. `length & 15` performs better.


@qeesung yes，i think so.
PR welcome

> Am 29.06.2019 um 08:15 schrieb xiaoheng1 <notifications@github.com>:
> 
> @qeesung yes，i think so.
> 
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub, or mute the thread.

@normanmaurer Ok, I will submit a PR.
@qeesung 
> length & 15 performs better.

I'm not that sure: last time I've checked length % 16, the JIT, given that 16 is constant, is able to perform the same optimization you'll perform by hand. Please check the assembly or perform a benchmark to be sure of it

@franz1981 I will check it more deeply. :-)
I am pretty sure that HashMap uses &. As the HashMap is very optimized I think & might be faster 
> I am pretty sure that HashMap uses &. As the HashMap is very optimized I think & might be faster

I found the code in the JDK7 source code

https://github.com/openjdk-mirror/jdk7u-jdk/blob/f4d80957e89a19a29bb9f9807d2a28351ed7f7df/src/share/classes/java/util/HashMap.java#L272-L277

```java
/**
* Returns index for hash code h.
*/
static int indexFor(int h, int length) {
    return h & (length-1);
}
```
Maby they also used this method because length may not be constant? 
@SplotyCode @qeesung The JVM doesn't trust final (non static) fields unless for specific internal java util (invoke) classes ie length cannot be assumed (from the JIT) as constant on both `ArrayDeque` and `IdentityHashMap` or other impls, that need to use the "clever" mask bit-trick instead. 
`prettyHexDump` is hardcoding as constant 16 so this kind of trick is not needed and should be avoided as "premature" optimization unless there are strong proof that the JVM has failed to optimize it.

Pull request review comments:

Patch that resolved the issue:
diff --git a/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java b/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java
index ae8d9ed3ea2..b8fb5b446f7 100644
--- a/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java
+++ b/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java
@@ -1095,7 +1095,7 @@ private static String prettyHexDump(ByteBuf buffer, int offset, int length) {
             if (length == 0) {
               return StringUtil.EMPTY_STRING;
             } else {
-                int rows = length / 16 + (length % 15 == 0? 0 : 1) + 4;
+                int rows = length / 16 + ((length & 15) == 0? 0 : 1) + 4;
                 StringBuilder buf = new StringBuilder(rows * 80);
                 appendPrettyHexDump(buf, buffer, offset, length);
                 return buf.toString();


Test patch: