xylz,imxylz

关注后端架构、中间件、分布式和并发编程

   :: 首页 :: 新随笔 :: 联系 :: 聚合  :: 管理 ::
  111 随笔 :: 10 文章 :: 2680 评论 :: 0 Trackbacks
线上服务器负载过高发生了报警,同事找我求救。
我看到机器的负载都超过20了,查看java进程线程栈,找到了出问题的代码。

下面是其代码片段,实际情况错误处理比这更坏。
 1 package demo;
 2 
 3 import java.io.BufferedReader;
 4 import java.io.InputStream;
 5 import java.io.InputStreamReader;
 6 import java.net.HttpURLConnection;
 7 import java.net.URL;
 8 import java.net.URLConnection;
 9 import org.apache.commons.lang.StringUtils;
10 
11 /**
12  * @author adyliu (imxylz#gmail.com)
13  * @since 2012-3-15
14  */
15 public class FaultDemo {
16 
17     /**
18      * @param args
19      */
20     public static void main(String[] args) throws Exception {
21         final String tudou = "http://v.youku.com/v_playlist/f17170661o1p9.html";
22 
23         URL url = new URL(tudou);
24         HttpURLConnection conn = (HttpURLConnection) url.openConnection();
25         conn.connect();
26         try {
27             InputStream in = conn.getInputStream();
28             BufferedReader br = new BufferedReader(new InputStreamReader(in, "utf-8"));
29             StringBuilder buf = new StringBuilder();
30             String line = null;
31             while ((line = br.readLine()) != null) {
32                 if (StringUtils.isNotEmpty(buf.toString())) {
33                     buf.append("\r\n");
34                 }
35                 buf.append(line);
36             }
37             //do something with 'buf'
38 
39         } finally {
40             conn.disconnect();
41         }
42 
43     }
44 
45 }
46 

思考下,这段代码有什么致命问题么?(这里不追究业务逻辑处理的正确性以及细小的瑕疵)
.
..
...
现在回来。
我发现线程栈里面的线程都RUNNABLE在32行。
这一行看起来有什么问题呢?StringBuilder.toString()不是转换成String么?Apache commons-lang里面的StringUtils.isNotEmpty使用也没问题啊?
看代码,人家的逻辑其实是判断是否是第一行,如果不是第一行那么就增加一个换行符。

既然CPU在这里运行,那么就说明这个地方一定存在非常耗费CPU的操作,导致CPU非常繁忙,从而系统负载过高。
看详细堆栈,其实CPU在进行内存的拷贝动作。
看下面的源码。
java.lang.StringBuilder.toString()
    public String toString() {
        // Create a copy, don't share the array
    return new String(value, 0, count);
    }
接着看java.lang.String的构造函数:
    public String(char value[], int offset, int count) {
        if (offset < 0) {
            throw new StringIndexOutOfBoundsException(offset);
        }
        if (count < 0) {
            throw new StringIndexOutOfBoundsException(count);
        }
        // Note: offset or count might be near -1>>>1.
        if (offset > value.length - count) {
            throw new StringIndexOutOfBoundsException(offset + count);
        }
        this.offset = 0;
        this.count = count;
        this.value = Arrays.copyOfRange(value, offset, offset+count);
    }

看出来了么?
问题的关键在于String构造函数的最后一行,value并不是直接指向的,而是重新生成了一个新的字符串,使用系统拷贝函数进行内存复制。
java.util.Arrays.copyOfRange(char[], int, int)
    public static char[] copyOfRange(char[] original, int from, int to) {
        int newLength = to - from;
        if (newLength < 0)
            throw new IllegalArgumentException(from + " > " + to);
        char[] copy = new char[newLength];
        System.arraycopy(original, from, copy, 0,
                         Math.min(original.length - from, newLength));
        return copy;
    }

好了,再回头看逻辑代码32行。
if (StringUtils.isNotEmpty(buf.toString())) {
    buf.append("\r\n");
}
这里有问题的地方在于每次循环一行的时候都生成一个新的字符串。也就是说如果HTTP返回的结果输入流中有1000行的话,将额外生成1000个字符串(不算StringBuilder扩容生成的个数)。每一个字符串还比前一个字符串大。


我们来做一个简单的测试,我们在原来的代码上增加几行计数代码。
    int lines =0;
    int count = 0;
    int malloc = 0;
    while ((line = br.readLine()) != null) {
        lines++;
        count+=line.length();
        malloc += count;
        if (StringUtils.isNotEmpty(buf.toString())) {
            buf.append("\r\n");
        }
        buf.append(line);
    }
    System.out.println(lines+" -> "+count+" -> "+malloc);
我们记录下行数lines以及额外发生的字符串拷贝大小malloc。
这是一次输出的结果。
1169 -> 66958 -> 39356387
也就是1169行的网页,一共是66958字节(65KB),结果额外生成的内存大小(不算StringBuilder扩容占用的内存大小)为39356387字节(37.5MB)!!!
试想一下,CPU一直频繁于进行内存分配,机器的负载能不高么?我们线上服务器是2个CPU 16核,内存24G的Redhat Enterprise Linux 5.5,负载居然达到几十。这还是只有访问量很低的时候。这就难怪服务频繁宕机了。

事实上我们有非常完善和丰富的基于Apache commons-httpclient的封装,操作起来也非常简单。对于这种简单的请求,只需要一条命令就解决了。
String platform.utils.HttpClientUtils.getResponse(String)
String platform.utils.HttpClientUtils.postResponse(String, Map<String, String>)

即使非要自造轮子,处理这种简单的输入流可以使用下面的代码,就可以很好的解决问题。
    InputStream in = 
    ByteArrayOutputStream baos = new ByteArrayOutputStream(8192);
    int len = -1;
    byte[] b = new byte[8192];//8k
    while ((len = in.read(b)) > 0) {
        baos.write(b, 0, len);
    }
    baos.close();//ignore is ok
    String response =  new String(baos.toByteArray(), encoding);

当然了,最后紧急处理线上问题最快的方式就是将有问题的代码稍微变通下即可。
    if (buf.length() > 0) {
        buf.append("\r\n");
    }


这个问题非常简单,只是想表达几个观点:
  • 团队更需要合作,按照规范来进行。自造轮子不是不可以,但是生产环境还是要限于自己熟悉的方式。
  • 即使非常简单的代码,也有可能有致命的陷阱在里面。善于思考才是王道。
  • 学习开源的代码和常规思路,学习解决问题的常规做法。这个问题其实非常简单,熟悉输入输出流的人非常熟练就能解决问题。


©2009-2014 IMXYLZ |求贤若渴
posted on 2012-03-15 18:30 imxylz 阅读(11446) 评论(16)  编辑  收藏 所属分类: J2EE

评论

# re: 一次简单却致命的错误[未登录] 2012-03-15 22:06 Joey
if (buf.toString().length() > 0) {
buf.append("\r\n");
}
既然是toString() 方法 每个循环 都会生成新的String对象引起的为什么还要像上面那样解决?  回复  更多评论
  

# re: 一次简单却致命的错误 2012-03-15 22:10 xylz
@Joey
谢谢提醒,写错了。
直接用文本复制粘贴手动修改错了。去掉toString()就可以了。  回复  更多评论
  

# re: 一次简单却致命的错误[未登录] 2012-03-16 09:40 changedi
嗯,在技术汪洋中,我们很容易陷入而忽略了那些基本的思考~  回复  更多评论
  

# re: 一次简单却致命的错误 2012-03-16 11:03 小明
写出 if (StringUtils.isNotEmpty(buf.toString()))的程序员应该裁掉  回复  更多评论
  

# re: 一次简单却致命的错误[未登录] 2012-03-16 11:27 kevin
@小明
同意,为什么总有人喜欢写StringUtils之类的东西,我在很多项目里都看到过。  回复  更多评论
  

# re: 一次简单却致命的错误 2012-03-16 18:35 Saga
@changedi
确实,深有感受  回复  更多评论
  

# re: 一次简单却致命的错误 2012-03-17 15:17 bescq
在三分钟内看出这个代码问题,并给与解决方案的人,在贵司能给个什么级别?大概值多少米?   回复  更多评论
  

# re: 一次简单却致命的错误 2012-03-19 19:36 路过
@bescq
8K-1W  回复  更多评论
  

# re: 一次简单却致命的错误 2012-03-22 00:14 iamct
亲,偶尔瞅了一眼排行。第6:) 另外,为啥不把那个判断放在循环外面呢?  回复  更多评论
  

# re: 一次简单却致命的错误 2012-03-27 17:34 new comer
"我发现线程栈里面的线程都RUNNABLE在32行。",大侠,能否讲一下这个是怎样发现的?  回复  更多评论
  

# re: 一次简单却致命的错误 2012-03-27 17:35 new comer
"我发现线程栈里面的线程都RUNNABLE在32行。"

大侠能否讲一下这个是怎样发现的?  回复  更多评论
  

# re: 一次简单却致命的错误[未登录] 2012-03-30 19:23 y
@new comer
应该是线程栈 打出来 一眼扫过去 一部分线程正在执行的代码都是一个地方  回复  更多评论
  

# re: 一次简单却致命的错误 2012-12-20 11:53 dohkoos
if (StringUtils.isNotEmpty(buf.toString()))
写出这断代码是对StringUtils中的方法还不熟悉,isNotEmpty中的参数是CharSequence类型,不需要转换的。
  回复  更多评论
  

# re: 一次简单却致命的错误 2015-01-18 12:40 风车
同意@小明
  回复  更多评论
  

# re: 一次简单却致命的错误 2015-06-16 09:49 高帆
请交大侠!查看java线程是怎么看的  回复  更多评论
  

# re: 一次简单却致命的错误 2015-11-16 11:52 shaw
@高帆
jstack 打印出来 线程栈信息,能看到 线程栈目前运行在那个地方,等待什么资源  回复  更多评论
  


只有注册用户登录后才能发表评论。


网站导航:
 

©2009-2014 IMXYLZ