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

Optimize shouldComponentUpdate #96

Merged
merged 2 commits into from
Nov 2, 2016
Merged

Optimize shouldComponentUpdate #96

merged 2 commits into from
Nov 2, 2016

Conversation

yesmeck
Copy link
Member

@yesmeck yesmeck commented Nov 1, 2016

  1. 把所有 columns 相关的计算都抽出来,并对计算结果进行缓存,避免每次都生成新的 columns 对象;
  2. 不直接给 TableRow 传 style,避免每次都生成新的 style 对象。

这样改完后发现 #95 其实不需要了, shouldComponentUpdate 可以正常工作了。

@afc163
Copy link
Member

afc163 commented Nov 1, 2016

给力,搞个性能对比看看效果?

@@ -4,6 +4,7 @@ import TableHeader from './TableHeader';
import { measureScrollbar, debounce } from './utils';
import shallowequal from 'shallowequal';
import addEventListener from 'rc-util/lib/Dom/addEventListener';
import ColumnManager from './ColumnManager';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

文件名大小写。

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

调整了。

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.7%) to 80.497% when pulling 09c08d0 on yesmeck:optimize into ea2dc4b on react-component:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.7%) to 82.824% when pulling fb681ad on yesmeck:optimize into ea2dc4b on react-component:master.

@yesmeck
Copy link
Member Author

yesmeck commented Nov 1, 2016

简单测试了下,用下面的代码渲染 100 条数据。

const React = require('react');
const ReactDOM = require('react-dom');
const Table = require('rc-table');
require('rc-table/assets/index.less');

const columns = [{
  title: '姓名',
  dataIndex: 'name',
  width: 150,
  fixed: 'left',
  key: 'title',
}, {
  title: '年龄',
  dataIndex: 'age',
  width: 150,
  key: 'age',
}, {
  title: '住址',
  dataIndex: 'address',
  key: 'address',
}];

const data = [];
for (let i = 0; i < 100; i++) {
  data.push({
    key: i,
    name: `李大嘴${i}`,
    age: 32,
    address: `西湖区湖底公园${i}号`,
  });
}

ReactDOM.render(<Table columns={columns} data={data} />, document.getElementById('__react-content'));

在 Table 的 render 第一行加上 console.time('render');,componentDidUpdate 最后行加上 console.timeEnd('render');

不加这个 patch 前鼠标 hover 一次 Table render 一次的时间大概是 60ms 左右,加上这个 patch 后大概在 15-20ms。然后顺便试下了 #95 的方案,因为这个方案在 hover 的时候 Table 不需要渲染,所以渲染 hover 那行 TableRow 只要零点几毫秒。

@afc163 另外你们之前有没有写 React benchmark 的方案?

@yesmeck
Copy link
Member Author

yesmeck commented Nov 1, 2016

感觉 #95 的方案还是可以考虑下的。

@afc163
Copy link
Member

afc163 commented Nov 1, 2016

@afc163
Copy link
Member

afc163 commented Nov 1, 2016

#95 也合并了好了,发个 5.1

@yesmeck
Copy link
Member Author

yesmeck commented Nov 1, 2016

对一行 row 做一次 hover 和 unhover,Perf 的数据:

master

master

每行 TableRow 都被重新渲染,总共渲染了400 次。

这个 PR

optimize

只有 hover 的那行 TableRow 重新渲染了,但是渲染 Table 还是需要 20ms 左右。

#95

95

只渲染了 TableRow,只要零点几毫秒。

@yesmeck
Copy link
Member Author

yesmeck commented Nov 2, 2016

@afc163 没问题合并了?

@yizhengfeng-jj
Copy link

yizhengfeng-jj commented Dec 2, 2019

请问这个columns里面的render内容如果用组件代替的话,但是每次切换页数,像@yesmeck说的shouldComponent会重新走一遍,确切的说是组件会重新加载一次因为componentDidMount也执行了,这个问题是2016年提出来的,现在是2019/12 我的antd版本是antd:"^3.18.1",我测了一下还是有这个问题,这个问题解决了么现在,可以回复一下么,万分感谢

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

Successfully merging this pull request may close these issues.

None yet

4 participants