一个静态编译修改引发的性能问题

一次不经意的CodeReview，发现了一段奇怪的代码，追根溯源，在项目组内部引发了一场激烈的讨论。

讨论过后，问题得以纠正，但是这个问题背后蕴含的深层次问题，值得深思。

问题代码示例如下 ：原始第一版代码如下所示：

在合代码时，发现有代码冲突，检视代码之后，发现body()方法被修改了，修改之后的第二版代码如下所示：

从功能上看，这段代码是Restful协议栈相关的代码，比较底层，同时功能也很稳定，正常情况下不应该被随意修改。

查看Git提交记录，与修改人进行交流后得知，这段代码的修改原因竟然是CI静态检查（FindBugs/PMD等）报错，按照公司要求，静态检查相关的问题要清零，于是按照静态检查工具的建议进行了修改。

报错的原因大致如下：如果直接返回[] body，这个对象可能会被引用者修改，为了防止被修改，所以需要每次获取时Copy一个新的byte数组出来，修改如下：

类似这样的修改，在整个项目中还存在多处，都是类似的原因和问题。

表面看，这个修改从功能上看没有任何问题，功能测试用例也能跑过，但是却存在如下几个问题：

1、这是网关平台的Restful协议栈，每次API调用都需要获取Body并在应用层做反序列化。如果每次调用都额外拷贝一次，对性能的影响比较大。

2、byte数组的申请和释放相对比较耗时，需要避免频繁新建和销毁，但是上述修改却反其道而行之。

3、对于调用方而言，通过属性的get方法获取对象通常获得的都是引用，而非克隆或者拷贝。这个是编程惯例，但是上述修改打破了这个惯例，在底层做了隐性拷贝，对调用方却不可见，这个是非常糟糕的玩法。

总结起来，修改之后的代码功能上没问题，但是在性能和GC上却存在巨大的隐患，下面我们通过Demo来模拟测试下。

对GC的影响：

频繁、大量的创建生命周期较短的byte数组对象，会给新生代的GC带来巨大压力，来不及回收的内存将会被转移到老年代，等待Full GC释放。

测试代码如下（原版）：

GC日志：在循环1024次引用调用期间，没有发生GC（包括新生代）

内存：无显著增长和波动

测试代码（数组拷贝版）：

GC日志：在循环1024次引用调用期间，新生代内存申请失败，发生多次GC（YoungGen）

内存：明显的波动

看完GC影响之后，再看性能差异：

公共测试代码部分：

一个不经意的静态编译问题修改，却引发了严重的性能问题，在这次代码修改中，存在如下几个问题值得反思：

1、静态检查本来是用于解决问题，却因为修改不当引入了新的问题。对于工具和规则，不要盲从，需要辩证的看，而不是机械的执行。

2、Java基本功问题，显然，这个问题的修改非常业余。首先看下JDK，如果你需要修改引用对象本身，往往要实现clone方法，如下所示：

再以Netty的ByteBuf为例，提供了不同的拷贝方法，用于修改复制的引用对象，而不影响原始对象：

显然，如果按照我们的检查规则,JDK源码也是需要修改的...

3、静态检查问题修改往往按照分责任田的方式进行，例如一类问题，都统一由某一个人（或几个人）修改。由于修改者对非自己开发的模块代码并不熟悉，如果一味按照工具提示进行修改，就可能会引入新的问题。

这个问题折射出更深层次的原因是大家忙于解决表面看到的问题，忙于各种统计指标、项目进度，而忽略了代码质量、性能原则等...这才是真正的危险