一次排除线上for循环引起的游戏卡顿问题过程

发生问题

某年某月某日，正在公司划着水，吃着火锅唱着歌的时候，测试大佬突然反馈，外网玩家投诉说游戏卡出翔了！！！于是放下手中的筷子。。啊不是手机，淡定的登录了监控后台看了看，发现一切正常，不就垃圾回收次数变多了点嘛，cpu又还没到百分之一百，内存也还没到顶呢，怎么可能卡，一定是玩家无理取闹，测试瞎几把乱反馈。刚想骂回去怎么可能卡，想着我的GM账号好久没登录去虐玩家了，顺便上去看看吧，一顿操作猛如虎之后，顺利登陆了游戏，加载中，心里想等会就虐死你们这帮不充钱的白嫖党。加载完了，顺利进入游戏，哎，哎不是，怎么图标加载了2s多才显示出来。没事没事，可能遇上玄学卡顿了，没事游戏还能继续正常玩，哎，怎么刚才战斗好像卡了一下，哎我艹，怎么血量飘字隔了个几秒才飘出来。顿时觉得不妥，应该是有卡顿了，但是问题不大，只是偶尔卡一下那种，并没有造成游戏整体卡顿，猜测应该是有某个业务比较耗时，但是并没有产生死循环什么的导致死机的操作，还好问题不严重。

开始定位问题

于是淡定的打开了性能日志，查看是哪个协议导致卡顿。打开日志发现，玩家的某个功能的红点判断居然跑了5000毫秒，也就是5秒。难怪一上线，icon卡了那么久才出来，原来是icon内置红点逻辑卡顿。因为场景内玩家都是同一条线程，所以才会发生偶尔卡一下的情况，因为其他玩家占用了cpu。但是又不是死循环等情况，并没有所有线程卡死，只是卡几秒，导致监控看不出太大的异常，只是总体cpu有所上升。但是从目前的性能日志来看，也仅仅是记录了红点处理耗时过长，并不能知道具体是红点地下哪个业务有问题。因为红点地下套了好几层，类似于下面代码的结构。

public boolean vlaidReddot() {
	// 示例代码
	boolean flag1 = xxxService.validReddot()1;
	boolean flag2 = xxxService.validReddot()2;
	// xxxService.validReddot()方法下面可能还有其他的分支
	return flag1 || flag2;
}

因为底下代码分支过多，所以并不能很清晰的定位到具体是红点下哪个业务有问题。第一反应是开发环境下面试试会不会卡，不跑不知道，跑起来比德芙还要丝滑流畅，而且对应的代码性能快的飞起，并不像线上一般慢。那没办法了，只能开始猜测卡顿的原因，根据以往排查线上卡顿问题的经验来看，猜测业务代码中可能有调用了全服玩家判断相关的代码（因为全服玩家很多，不管调全服玩家出来干嘛，都不会是一个很快的操作，而且在经常调用的方法里面调了全服玩家的话，那就更爆炸），或者有绕过缓存直接从数据库里面读取大量数据的操作。但是后者缓存已经被改造成无法绕过了，所以第一直觉判断是调用了全服玩家相关接口。接下来就很简单了，直接review代码，猜测哪里有调用全服玩家相关的方法。
但是reivew了代码之后，居然没有发现有调用全服玩家相关的方法，也就是说，这是一个全新的导致卡顿的bug，和以往的问题不同，所以也就意味着，要重新去猜测导致卡顿的原因。而且看了代码提交记录之后，发现这一段代码已经好几百年没有人动过了，也就是说，要么是因为其他原因引起的卡顿，要么是这段代码原本就有性能问题，只是一直没有暴露出来，今天因为某种契机把问题暴露了。于是加了更加详细的日志，最终根据日志定位到是有好几段代码执行耗时过长，我节选了其中一段代码，简写如下：

public boolean validWearEquipReddot(Player player) {
	// 第一层循环，根据所有装备部位来循环
	for (EquipPosition position : GoodsPosition.getEquipList()) {
		// 第二层循环，遍历玩家所有的角色
		for (Role role : player.getRoleList()) {
			Equipment wearingEquip = role.getEquipByPosition(position);
			// 第三层循环，遍历玩家背包里面所有装备
			for (Equipment backpackEquip : player.getAllBackpackEquipmentsByPositions(GoodsPosition.getEquipList())) {
				// 这里面用backpackEquip和wearingEquip进行各种if的对比，我这里就简写了
				boolean result = wearingEquip == backpackEquip;
				if (result) {
					return true;
				}
			}
		}
	}
	return false;
}

问题超乎想像

从日志记录来看，就是类似于上图的这个代码执行耗时比较长，占用了约2秒。于是开始分析以上代码，第一层循环，是遍历所有的装备部位，这是一个常量的for循环，因为装备部位就那么多，只有8个，第一层for循环的次数是可控的。来到第二层for循环，这一层for循环是遍历玩家所有角色，但是玩家的角色最多3个，也就是说第二层for循环最多循环三次，比第一层循环还要少，故而不会因为第二层for循环导致卡顿。转而看第三层for循环，看到了点端倪，第三层for循环每次开始之前，都要去背包里面把所有装备找出来。虽然说这个操作不会很耗时，但是仍然是一个不妥的操作，理论上只需要从背包遍历一次所有装备就好了。第三层for循环里面，都是简单的if判断，理论上不会有性能问题。
我顿时就懵了，从代码上面来看，好像不是因为这段代码引起的性能问题啊，首先循环次数可控，循环次数最多是83背包最大数量，而且里面也没有耗时的业务啊，都是简单判断，最耗时的也就是player.getAllBackpackEquipmentsByPositions(GoodsPosition.getEquipList())这个遍历背包里面所有的装备了。尼玛这是为毛会卡啊，难道真的是随着时间的增加，游戏的主要矛盾转化为玩家日益增长的装备数量和落后的代码性能之间的矛盾吗？问题越发朴素迷离。没办法，只能本着死马当做活马医的想法，去改造上面这段代码。本来也是有一丢丢问题的嘛。

改造之后，无意之间发现掩藏在重重迷雾之间的根源

改造之后，代码如下：

public boolean validWearEquipReddot(Player player) {
	List<EquipPosition> positions = GoodsPosition.getEquipList();
	List<Equipment> equips = player.getAllBackpackEquipmentsByPositions(positions);
	
	for (Equipment backpackEquip : equips) {
		for (Role role : player.getRoleList()) {
			Equipment wearingEquip = role.getEquipByPosition(backpackEquip.getPosition());
			// 这里面用backpackEquip和wearingEquip进行各种if的对比，我这里就简写了
			boolean result = wearingEquip == backpackEquip;
			if (result) {
				return true;
			}
		}	
	}
	return false;
}

咋一眼看上去，好像减少了for循环，其实本质上和之前的代码差不了多少，最主要的区别是，把循环每次都要从背包里面获取当前的装备，改成了方法一开始的时候就获取背包所有装备，然后缓存起来直到方法结束。理论上来说会减少一定次数的循环，只能期待问题真的是这个。
代码改好，总归要自测一下改造之后的代码逻辑是否正常吧，于是打开断点调试，模拟了集中判断红点的情况之后。嗯，好像代码牛的一批，根本没问题。本着负责的理念，自测多两次，继续打断点观察代码运行的结果。突然之间，鬼使神差，把鼠标移到了positions这个列表上面，idea显示出了这个列表的内容。诶，这个列表好像有点长耶。再仔细看看，what！！！，怎么好像有几个是重复部位。这个时候我才反应过来，我操，有业务对这个列表进行了改动，这个理应为不可改变的列表被别人改变了！！！于是马上查看GoodsPosition.getEquipList()返回列表的引用，发现有地方的代码是这样子的：

public void logEquip() {
	// GoodsPosition.getEquipList()返回的是一个ArrayList
	List<EquipPosition> positions = GoodsPosition.getEquipList();
	// 这里是另外一种装备的部位，实例代码省略获取过程
	List<EquipPosition> anthoerPositions = new ArrayList<>();
	positions.addAll(anthoerPositions);
	// 接下来是具体的业务处理
}

原来导致卡顿的问题是在这里面，GoodsPosition.getEquipList()本应该返回来的是一个不可修改的列表，但是随着每一次调用logEquip，此列表越加越多越加越多，导致红点判断的for循环，有一个可控次数的for循环，变成了一个随时时间的增长，循环次数越来越多的循环，从而导致代码执行时间过长。问题终于定位到了，开发环境修复，发布，搞定。

问题总结

其实这是一个非常简单的问题，可能由于我上面写的比较简单，可能认为问题比较小。其实这个问题，前前后后大概耗费了一个月时间，不断的加日志去观察，一步步定位到问题的代码段，反复猜测有问题的代码是哪些。甚至当初定位到上面的for循环执行耗时过长的时候，一开始review完代码，并不觉得问题是出现在这里，而是认为在运行for循环的时候，恰好发生了GC导致for循环执行时间过长而已，并不是这段代码本身有问题。总而言之，因为错误的猜测，和还要做游戏版本迭代的其他工作，前后是耗费了比较长的时间去处理的。还好是一开始出现问题的时候，用热更绕过了这段代码，让玩家接下来玩能不再卡顿，才有了更多的时间去排查问题。
那问题的原因也很简单，同事A在编写GoodsPosition.getEquipList()这个方法的时候，根本没有想到别人会改这个list，所以就没用Collections.unmodifiableList()去包装返回的list让它不可改变。然后同事B在调用这个方法的时候，也没有注意正规使用，直接改变了别的模块负责的容器。很简单的一个沟通协作问题，问题的起源就是两个人都不够细心，导致了一次可以说是重大级别的线上卡顿问题。

严格要求自己，不要轻视业务

有人会问我，程序员如何提高自己的技术水平。其实完善自己的细节编码，其实就是一种很大的提高。我在平时review别人代码的时候，如果漏了private修饰符，我都需要他加上去，然后跟他说这么一句话：不要养成这种遗漏的细小细节的坏习惯，要对自己严格。因为，这就是你的提升，只有你对自己严格，只有你写的代码足够健壮，不管别人怎么折腾都不会有问题，在上司眼中你才有资格提高水平。毕竟公司是一个团队，不是单打独斗，合作沟通能力也是自己的一种水平。
很多人对写业务嗤之以鼻，所以在做业务的时候不认真对待，瞎几把写完就算求。但是，能正确理解策划的需求，并且正确的编写业务逻辑，认真对待，不敷衍了事，也是一种自我能力的突破。当自己觉得写业务的时候很无聊的时候，不妨思考一下，编码上面有什么可以考虑的地方，例如扩展性，策划以后加需求怎么办；健壮性，别人调用我的接口参数有问题怎么办；便于理解性，策划的这个逻辑有点妖娆，能不能在编码上面写的清晰一点，备注多一点。当你的上司觉得你写基础业务写的很好的时候，才会让你去做一些高级的业务，去接触底层，甚至是搭建跨服服务器及其跨服逻辑。不然，就算自己会再多的框架，读了再多的源码，自研的框架多么牛逼，但是在实际的编码过程中，因为漏这漏那，不断的去修一些小bug的时候，那么在你上司的眼中，你就是一个连业务都写不好的菜鸡，水平能好得到哪里。我相信的一句话是，一个连业务都写不好的程序员，技术再好再牛逼也是不合格的程序员。