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

[Bug] 每次初始化表格渲染时,会调用四次Sum函数,每次选择单元格会执行两次Sum函数 #1333

Closed
2 tasks done
iamyunsin opened this issue Feb 4, 2024 · 7 comments · Fixed by #1353
Assignees
Labels
bug Something isn't working

Comments

@iamyunsin
Copy link

初始清单

  • 这真的是个问题吗?
  • 我已经在 Github Issues 中搜索过了,但没有找到类似的问题。

受影响的包和版本

dev

复现步骤

  1. 修改examples/src/sheets/main.ts文件,初始化空白sheet,变更代码如下:
// univer.createUniverSheet(DEFAULT_WORKBOOK_DATA_DEMO);
univer.createUniverSheet({});
  1. 运行pnpm dev:demo命令启动开发服务
  2. 打开浏览器,访问http://localhost:3002,并在packages/engine-formula/src/functions/math/sum/index.ts文件中的第24行打上断点
  3. 刷新当前页面,发现执行了四次sum函数
  4. 当单击单元格切换选区或新增工作表,或修改任意单元格的样式(不是值),都会触发执行两次Sum函数
  • 额外说明:
    我通过查看调用堆栈,发现是在packages/sheets-ui/src/controllers/status-bar.controller.ts文件的第145行触发的。

预期行为

  1. 初始化时候只执行1次Sum函数
  2. 切换worksheet时执行1次Sum函数
  3. 仅仅改变选区和单元格样式时,不执行Sum函数
  4. 修改单元格值时候执行1次Sum函数`

实际行为

  1. 初始化时执行了4次Sum函数
  2. 切换worksheet时执行了2次Sum函数
  3. 修改选区和修改单元格样式的时候,执行了2次Sum函数
  4. 修改单元格值时,执行了2次Sum函数

运行环境

Chrome

操作系统

macOS

构建工具

esbuild, Vite

@iamyunsin iamyunsin added the bug Something isn't working label Feb 4, 2024
@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


Initial list

  • Is this really a problem?
  • I've searched Github Issues but haven't found any similar issues.

Affected packages and versions

dev

Reproduction steps

  1. Modify the examples/src/sheets/main.ts file, initialize the blank sheet, and change the code as follows:
//univer.createUniverSheet(DEFAULT_WORKBOOK_DATA_DEMO);
univer.createUniverSheet({});
  1. Run the pnpm dev:demo command to start the development service
  2. Open the browser, visit http://localhost:3002, and put a breakpoint on line 24 in the packages/engine-formula/src/functions/math/sum/index.ts file
  3. Refresh the current page and find that the sum function has been executed four times.
  4. When you click a cell to switch selections, add a new worksheet, or modify the style of any cell (not the value), the Sum function will be executed twice.

*Additional instructions:
By looking at the call stack, I found that it was triggered on line 145 of the packages/sheets-ui/src/controllers/status-bar.controller.ts file.

Expected behavior

  1. Only execute the Sum function 1 times during initialization
  2. Execute the Sum function 1 times when switching worksheets
  3. When only changing the selection and cell styles, the Sum function is not executed.
  4. When modifying the cell value, execute the Sum function 1 times

Actual behavior

  1. The Sum function was executed 4 times during initialization.
  2. The Sum function was executed 2 times when switching worksheets.
  3. When modifying the selection and cell style, the Sum function was executed 2 times
  4. When modifying the cell value, the Sum function was executed 2 times

Running environment

Chrome

operating system

macOS

Build tools

esbuild, Vite

@Dushusir
Copy link
Member

Dushusir commented Feb 6, 2024

@yuhongz 帮忙看看是否有status bar 的计算是否有优化空间。

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


The formula calculation does currently have a problem with too many calls, and will be optimized later.

@yuhongz Please help me see if there is room for optimization in the calculation of the status bar.

@yuhongz
Copy link
Contributor

yuhongz commented Feb 7, 2024

发现不必要的计算,已在这个 PR #1353 优化掉了。
修改样式会导致重新计算是在预期中的行为,因为修改样式和修改值都是使用 SetRangeValueMutation,要区分开来需要用更细粒度的比较,也会带来性能负担

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


Unnecessary calculations were found and have been optimized away in this PR #1353.
Modifying the style will cause recalculation, which is expected behavior, because both modifying the style and modifying the value use SetRangeValueMutation. To distinguish them, you need to use a more fine-grained comparison, which will also bring a performance burden.

@iamyunsin
Copy link
Author

发现不必要的计算,已在这个 PR #1353 优化掉了。 修改样式会导致重新计算是在预期中的行为,因为修改样式和修改值都是使用 SetRangeValueMutation,要区分开来需要用更细粒度的比较,也会带来性能负担

但是status bar是公共能力,如果表格数据特别多的情况下,应该会产生CPU资源浪费和性能问题(即使计算都放到Web Worker中,也可能产生性能问题,看看是否需要构造一个大数据量的表格来做性能测试吧)。

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


Unnecessary calculations were found and have been optimized in this PR #1353. Modifying the style will cause recalculation, which is expected behavior, because both modifying the style and modifying the value use SetRangeValueMutation. To distinguish them, you need to use a more fine-grained comparison, which will also bring a performance burden.

However, the status bar is a public capability. If there is a lot of table data, it may cause a waste of CPU resources and performance problems (even if the calculations are all done in the Web Worker, performance problems may occur. Check whether you need to construct a large amount of data. Use the form to do performance testing).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants