代码优化：性能优化不是丑陋代码的遮羞布！

在编写代码的时候，我们往往要在多方面因素之中进行权衡。正确性毫无疑问是最基本的要求，这个都不需要讨论。而如何在可读性和性能之间进行权衡，确实是值得思考的问题。

我对于这方面的看法是：我更倾向于可读性为第一位。只有当代码的可读性好的时候，才容易维护，且不容易引起bug。并且当真的需要提高性能的时候，由于代码的良好的可读性，才便于分析代码，找出瓶颈，从而提高性能。

那么在不影响可读性的基础上，如何提高代码的性能呢？我认为第一步要做的是，审视系统架构，从软件到硬件，寻找可以进行提高的地方。比如，是否可以很好的支持并发，等等。第二，审视软件设计，寻找处理瓶颈，进行设计上的提高；第三，审视使用的算法，优化算法，提高性能。最后，在以上都无可作为的情况下，只能进行代码级别的优化时，一定要通过性能测试或者分析如profile等，找到真正的热点或者瓶颈。不然费了半天的劲，却只是事倍功半。而且在等进行代码级别的优化时，也要慎重优化，仔细的考量是否性能的提升程度，是否值得降低代码的可读性或者可维护性。是否有其它更好的方法解决。绝对不能以高性能作为丑陋代码的借口！

下面谈谈我这两天解决的一个Bug，也是引起我写这篇文章的原因。该Bug的现象是将某一型号的设备升级到mainline时，有时候核心进程会不断的crash。该模块不是我负责的模块，所以查了1天才找到问题。看到造成问题的代码，我实在是费解。

代码样例如下：

void opt_hash_index(u32 *index)

{

switch (g_mem_size) {

case HIGH_MEM_SIZE:

*index = *index & HIGH_MEM_MASK;

break;

case MEDIUM_MEM_SIZE:

*index = *index & MEDIUM_MEM_MASK;

break;

case LOW_MEM_SIZE:

*index = *index & LOW_MEM_MASK;

break;

case LOWEST_MEM_SIZE:

*index = *index & LOWEST_MEM_MASK;

break;

default:

break;

}

}

其中g_mem_size与设备型号相关，在系统启动阶段，根据不同型号而获得的资源限制。参数index为经过hash函数计算后的index，用于做hash表中桶的索引。

我在看到这个函数的时候，看到opt_hash_index是根据四种不同情况，才对index进行修改。最初的想法是该index只有在这四种情况下，才需要进行修改，其余情况则不需要修改。由于没有coredump文件，只有崩溃时的函数栈，且bug很难重现，所以我只能通过阅读代码来查找问题。但是在我看完计算index的hash算法后，发现该函数并没有保证index的大小一定小于桶的个数。

于是又重新查看opt_hash_index这个函数，查看各个case值和mask的宏定义：

#define HIGH_MEM_CNT 20

#define MEDIUM_MEM_CNT 19

#define LOW_MEM_CNT 18

#define LOWEST_MEM_CNT 17

#define HIGH_MEM_MASK ((1ul<<HIGH_MEM_CNT)-1)

#define MEDIUM_MEM_MASK ((1ul<<MEDIUM_MEM_CNT)-1)

#define LOW_MEM_MASK ((1ul<<LOW_MEM_CNT)-1)

#define LOWEST_MEM_MASK ((1ul<<LOWEST_MEM_CNT)-1)

#define HIGH_MEM_SIZE (1ul<<HIGH_MEM_CNT)

#define MEDIUM_MEM_SIZE (1ul<<MEDIUM_MEM_CNT)

#define LOW_MEM_SIZE (1ul<<LOW_MEM_CNT)

#define LOWEST_MEM_SIZE (1ul<<LOWEST_MEM_CNT)

看完定义，我有些疑惑了。按照这几个宏的定义，opt_hash_index不就是一个求模的运算吗？我真的不敢相信这一点。因为美国那边的工程师的水平还是不错的。我来来回回的检查了这几个宏，并查看了opt_hash_index的调用关系。最终还是确定这就是一个求模运算，用于限制index的大小，使其不得大于g_mem_size。那么造成问题的原因也很简单。负责底层平台资源的team中的开发人员，修改了这一平台的预定的资源个数。将其扩大为2*1024*1024，不再是这里的HIGH_MEM_SIZE的大小。因此opt_hash_index实际上不会对index做任何修改，以至于某些情况下，index越界从而使进程崩溃。

于是我修改了opt_hash_index函数，直接使用求模运算来限制index的大小。

void opt_hash_index(u32 *index)

{

*index %= g_mem_size;

}

但是由于我实在不能相信那边的工程师不知道求模运算，于是发邮件询问此事。第二天得到答复，原来使用switch..case..是为了追求性能，因为switch case的情况下，可以直接使用掩码。并且这种设计是经过那边的一位技术leader首肯的。然后他给出的意见是，或者让平台组修改资源个数，或者我们这边修改宏定义使之符合目前的资源个数。

当我得知这个原因时，真有些发疯了。这样的代码可读性和维护性简直是一塌糊涂啊！如果你只支持那四个固定的资源个数。拜托请加上一些注释，并且在default处使用BUG()开关，使代码运行到此处自动crash或者报警才是！再说了为了这一点点的性能提升，放弃了这么多，值的吗？！

于是我想看看switch..case究竟比mod能提高多少性能。毕竟switch case的实现一般都是使用跳转完成的，那么额外的比较也需要cpu的计算资源，更何况跳转会破坏CPU的pipeline，即流水线工作呢！

于是我写了下面的简单的测试程序：

#define LOOPS 100000000

int main(void)

{

int i;

g_mem_size = LOW_MEM_SIZE;

for (i = 0; i < LOOPS; ++i){

opt_hash_index1(i);

}

return 0;

}

注：我对原来的参数类型进行了修改，由u32* 变为u32

在使用switch case的情况下：

1. g_mem_size = HIGH_MEM_SIZE：0.413s

2. g_mem_size = MEDIUM_MEM_SIZE: 0.447s

3. g_mem_size = LOW_MEM_SIZE: 0.378s

4. g_mem_size = LOWEST_MEM_SIZE: 0.345s

使用mod运算时，跟g_mem_size大小无关，大约为0.403s

OK。switch case总体上是比mod这种方案性能要好一些。但是这是在循环1亿次的情况下，最快才提高不过0.06s。就这点性能的提升，值的写出这样天人共愤的代码！

我不想在这种性能优化与可读性的观点与其过多的争执。既然想要提升性能，那么我照样有好的方法，稍稍的降低一点可读性。

void opt_hash_index3(u32 index)

{

index &= (g_mem_size-1);

}

使用一个小技巧，“与”上"g_mem_size-1"来限制index不会超过g_mem_size的大小。

测试一下性能，其与g_mem_size的大小无关，约0.345s左右。

于是我今天回复美国那边，我不能接受以前的方案，可读性和维护性太差了！既然追求高性能，那么我可以使用方案三来取代mod运算。我将性能测试结果也告诉了他。方案三的性能既比switch case要好，可读性也要强得多。看看明天他如何回复。

其实，我为什么在开始的时候，直接使用mod而没有用方案三呢？还是因为可读性，执行一亿次才相差这么点，不值得牺牲可读性。

退一万步讲，即使有不错的性能提升，但是这种性能的提升也绝不是这种丑陋代码的遮羞布！完全可以使用各种方法来避免这种离奇的switch case。对此我不一一举例了。

我认为只有优美的代码才能保证质量，才能进行性能的提升！不要拿提高性能来为丑陋的代码遮羞！它不是一块遮羞布！！！