Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

并发文件处理可提速50%左右 #62

Open
zengjialuo opened this issue Nov 3, 2014 · 22 comments
Open

并发文件处理可提速50%左右 #62

zengjialuo opened this issue Nov 3, 2014 · 22 comments

Comments

@zengjialuo
Copy link
Member

目前 processor.process 这个方法是串行执行的。

实践得出,如果改成并发执行的话,整体构建的时间性能可以提升50%左右。

不知道这样做是不是好? 会否有其它隐患,比如,考虑 process 方法中有网络请求的情况?

@leeight
Copy link
Member

leeight commented Nov 3, 2014

估计要分processor来处理,像 css-compressor.js,js-compressor.js,less-compiler.js 之类的应该都没有问题

@otakustay
Copy link
Member

能知道这个并发快50%是快在哪里了吗?

如果我们从一开始就把所有文件拿出来放进内存里,之后所有processor都不再(除非有特殊情况)去使用fs模块读写硬盘上的文件,这个并发执行不知道还能不能有这么大的优势

文件一开始获取,processor不读硬盘这个是我一直觉得比较好的一个方案,现在有太多processor比较随意地读文件,有时候也会丢掉上一个processor的处理结果,之前就有遇到过这样的问题进行了修复

至于第一次获取所有文件,可以并发走,但要控制并发量,基本10个以上就会报系统错误

@zengjialuo
Copy link
Member Author

这个50%确实令人费解,理论上如果没有 io 操作,并行和串行都应该差不多……

@zengjialuo
Copy link
Member Author

噗,发现性能差异是源于串行的时候有一句setTimeout( nextFile, 1 );

改成setImmediate之后就好了

但是快的时间也很怪异,不是文件数量乘以某个常数时间,每个process都不一样

@otakustay
Copy link
Member

如果一个系统构建过程是重IO型的我就简直了……应该是CPU密集型才对- -

感觉确实有必要去梳理一下在build过程中涉及到的IO,要不要改并发基于此得到个最终的结论

@leeight
Copy link
Member

leeight commented Nov 5, 2014

其实beta版本里面改成 setImmediate 之后,貌似已经快了很多了。

@zengjialuo
Copy link
Member Author

嗯,一般的processor都是没多少io操作的,从这个角度看,并发意义不大。

之所以还是改了改,是考虑到有些特殊的 processor 是重 io 的(比如 bcsUploader,以及灰大提及的一些不规范的processor) ,如果不并发的话速度难以忍受。当然这些特殊的 processor 也可以通过改写父类的方法去实现并发(如我之前的做法),但这样做代码量大、重复,应该不值得被提倡。

另外,并发对于一般的 processor 似乎也没明显的坏处?

@leeight
Copy link
Member

leeight commented Nov 5, 2014

俺用 http://benchmarkjs.com/ 写一些 testcase 看看效果再说

@leeight
Copy link
Member

leeight commented Nov 5, 2014

貌似用 benchmark.js 测试出来的数据跟预期差别还是蛮大的。预期应该是差不多的,但是发现改成并发之后慢了不少

js-compiler x 319 ops/sec ±3.40% (87 runs sampled)
js-compiler-concurrent x 12.93 ops/sec ±2.80% (37 runs sampled)
Fastest is js-compiler
less-compiler x 301 ops/sec ±3.92% (71 runs sampled)
less-compiler-concurrent x 25.54 ops/sec ±3.35% (36 runs sampled)
Fastest is less-compiler

@otakustay
Copy link
Member

这个并发是怎么弄的,就是每个文件都setImmediate一下同时走?

@leeight
Copy link
Member

leeight commented Nov 5, 2014

用的async

@otakustay
Copy link
Member

看上去LessCompiler并没有涉及IO的内容,时间都花在进行任务队列的调度上面了?没IO的东西真不合适并发吧

@leeight
Copy link
Member

leeight commented Nov 5, 2014

那其实现在 edp-build 里面的,除了 module-compiler 之前,其它的都没有什么机会去读取本地文件的。

➜  lib git:(1.0/feature/62) ✗ ag fs.readFile .
processor/add-copyright.js (前面有条件判断)
45:        copyright = fs.readFileSync(fileLocation, 'utf8');

processor/tpl-merge.js (前面有条件判断)
254:        // 这里不应该调用 fs.readFileSync 而是应该从 processContext 中获取
260:                data         : fs.readFileSync( filePath ),

util/compile-module.js
52:                fs.readFileSync(depFile, 'UTF-8'),

@zengjialuo
Copy link
Member Author

benchmark.js 的测试结果有问题吧? 和我直接跑的结果差太远了……

另外是不是考虑下网络IO?

@otakustay
Copy link
Member

现在build过程哪些是会涉及网络IO的?LessCompiler肯定是没有的

@zengjialuo
Copy link
Member Author

@otakustay
Copy link
Member

这个不是已经用了异步并发了吗……?通过改造还能更好?

@zengjialuo
Copy link
Member Author

这个是通过覆盖父类的 processAll 方法,自己去并发的……如果父类本身是并发的话,就不用这么干了

@otakustay
Copy link
Member

所以问题回到最初,并发是不是真的能效提升性能,我把可能的结论归为3类:

  1. 在大部分或全部情况下能提升相当的性能,此时应该将processAll默认改为并发
  2. 在部分场景(如有网络IO)的情况下可以大幅度提升性能,其它场景下会降低性能,此时应该由父类提供processAllprocessAllParallel不同方法,或者使用一些属性控制,而不能一概而论转为并发
  3. 在大部分场景下会降低性能,仅少部分特殊场景有性能提升,此时保持现状由子类进行覆盖更为合理

是这样吗?

@zengjialuo
Copy link
Member Author

同意

@otakustay
Copy link
Member

@leeight 是不是考虑拿我这边的项目(规模够大),把几个Procssor一个一个改成异步的试试最终的实际效果,总结下哪些用异步有收益,哪些会出问题甚至没收益

然后我这边项目没用到的Processor,你这边应该也有项目会用到,也试上一次,拿个数据?我也感觉那个benchmark不是很准的样子

@leeight
Copy link
Member

leeight commented Nov 5, 2014

我也感觉挺怪的,那个测试其实也是处理100个less和100个js,我再用实际的项目测试一下看看吧,弄好之后发测试代码。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants