perf(core): ArrayField move method optimize by charlzyx · Pull Request #3863 · alibaba/formily

charlzyx · 2023-06-20T02:47:35Z

Before submitting a pull request, please make sure the following is done...

Ensure the pull request title and commit message follow the Commit Specific in English.
Fork the repo and create your branch from master or formily_next.
If you've added code that should be tested, add tests!
If you've changed APIs, update the documentation.
Ensure the test suite passes (npm test).
Make sure your code lints (npm run lint) - we've done our best to make sure these rules match our internal linting guidelines.

Please do not delete the above content

What have you changed?

性能优化：
ArrayField 中 move 方法，将 splice 替换为 for 循环处理，优化了超大数据场景下 ArrayTable 拖拽响应

优化前	优化后	#4263	说明
			Chrome Performance 截图
before-before.mp4	after-after.mp4	missing	录屏
before-before-Trace-20230620T103119.json.zip	after-after-Trace-20230620T102215.json.zip	with-length-Trace-20250116T181647.json.zip	profile

添加 #4263 改变后对比
测试环境:

Hardware Overview:
Model Name: MacBook Pro
Model Identifier: MacBookPro18,3
Model Number: MKGP3CH/A
Chip: Apple M1 Pro
Total Number of Cores: 8 (6 performance and 2 efficiency)
Memory: 16 GB

Chrome 版本 114.0.5735.133（正式版本） (arm64)

charlzyx · 2023-06-20T02:50:57Z

packages/core/src/__tests__/array.spec.ts

  expect(array.value).toEqual([1])
  array.move(0, 1)
-  expect(array.value).toEqual([1])
+  expect(array.value).toEqual([undefined, 1])


这个测试需要关注一下， [1] -> move(0,1) 原来结果为 [1], 现在结果为 [undefined, 1]

如果 undefined 不可被 move 的话，之前的 moveUp moveDown 应该也是不能够加入新的 undefined 值的
如果可以 move 的话， move(0, 1）的当前结果 [undefined, 1] 应该是正确的

这里要做个拦截的，如果没有移动条件，就不要新建数据

移动条件 怎么理解，是指 fromItem 不存在还是说 toItem 不存在，在原来的 test case 中
[1] -> moveUp(1) 结果为 [undefined, 1] , 在 moveUp(1) 会处理成 move(1, 0)

splice 版本中 [1] -> move(0,1) -> splice(0, 1), splice(1, 0, 1), 第二次 splice 的第一个 start 参数1 大于了数组长度

从而表现为最终结果还是 [1]

我是想说，move 的定位是移动，但不应该新增数据条目，这个不太符合预期吧

啊那如果这样的话，原来 moveUp 也是有问题的，原来 moveUp 的情况是
[1] -> moveUp(1) -> [undefined, 1]

那我一起 fixed 掉？

https://github.com/alibaba/formily/blob/formily_next/packages/core/src/models/ArrayField.ts#L131 这里有拦截逻辑，讲道理不会变成[undefined,1]的呀？

[1] -> moveUp(1) 比方说这个例子吧
return this.move(index, index - 1 < 0 ? this.value.length - 1 : index - 1)
->
return this.move(1, 0)

move(1, 0) 导致最终结果是 [undefined, 1]

codecov · 2023-06-20T02:55:38Z

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01 🎉

Comparison is base (1e49061) 96.58% compared to head (fee7600) 96.59%.

❗ Current head fee7600 differs from pull request most recent head 0a17322. Consider uploading reports for the commit 0a17322 to get more accurate results

Additional details and impacted files

@@               Coverage Diff                @@
##           formily_next    #3863      +/-   ##
================================================
+ Coverage         96.58%   96.59%   +0.01%     
================================================
  Files               152      152              
  Lines              6675     6695      +20     
  Branches           1801     1810       +9     
================================================
+ Hits               6447     6467      +20     
  Misses              228      228

Impacted Files	Coverage Δ
packages/core/src/models/ArrayField.ts	`100.00% <100.00%> (ø)`
packages/shared/src/array.ts	`100.00% <100.00%> (ø)`

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

charlzyx · 2023-06-20T02:55:51Z

测试用例

import React, { useMemo } from 'react'
import {
  FormItem,
  Input,
  ArrayTable,
  Editable,
  FormButtonGroup,
  Submit,
} from '@formily/antd'
import { createForm } from '@formily/core'
import { FormProvider, createSchemaField } from '@formily/react'

const range = (count: number) =>
  Array.from(new Array(count)).map((_, key) => ({
    a1: key,
  }))

const SchemaField = createSchemaField({
  components: {
    FormItem,
    Editable,
    Input,
    ArrayTable,
  },
})


const schema = {
  type: 'object',
  properties: {
    array: {
      type: 'array',
      'x-decorator': 'FormItem',
      'x-component': 'ArrayTable',
      'x-component-props': {
        pagination: { pageSize: 10 },
        scroll: { x: '100%' },
      },
      items: {
        type: 'object',
        properties: {
          column1: {
            type: 'void',
            'x-component': 'ArrayTable.Column',
            'x-component-props': { width: 50, title: 'Sort', align: 'center' },
            properties: {
              sort: {
                type: 'void',
                'x-component': 'ArrayTable.SortHandle',
              },
            },
          },
          column2: {
            type: 'void',
            'x-component': 'ArrayTable.Column',
            'x-component-props': { width: 80, title: 'Index', align: 'center' },
            properties: {
              index: {
                type: 'void',
                'x-component': 'ArrayTable.Index',
              },
            },
          },
          column3: {
            type: 'void',
            'x-component': 'ArrayTable.Column',
            'x-component-props': { width: 200, title: 'A1' },
            properties: {
              a1: {
                type: 'string',
                'x-decorator': 'Editable',
                'x-component': 'Input',
              },
            },
          },
          column4: {
            type: 'void',
            'x-component': 'ArrayTable.Column',
            'x-component-props': { width: 200, title: 'A2' },
            properties: {
              a2: {
                type: 'string',
                'x-decorator': 'FormItem',
                'x-component': 'Input',
              },
            },
          },
          column5: {
            type: 'void',
            'x-component': 'ArrayTable.Column',
            'x-component-props': { width: 200, title: 'A3' },
            properties: {
              a3: {
                type: 'string',
                'x-decorator': 'FormItem',
                'x-component': 'Input',
              },
            },
          },
          column6: {
            type: 'void',
            'x-component': 'ArrayTable.Column',
            'x-component-props': {
              title: 'Operations',
              dataIndex: 'operations',
              width: 200,
              fixed: 'right',
            },
            properties: {
              item: {
                type: 'void',
                'x-component': 'FormItem',
                properties: {
                  remove: {
                    type: 'void',
                    'x-component': 'ArrayTable.Remove',
                  },
                  moveDown: {
                    type: 'void',
                    'x-component': 'ArrayTable.MoveDown',
                  },
                  moveUp: {
                    type: 'void',
                    'x-component': 'ArrayTable.MoveUp',
                  },
                },
              },
            },
          },
        },
      },
      properties: {
        add: {
          type: 'void',
          'x-component': 'ArrayTable.Addition',
          title: 'Add entry',
        },
      },
    },
  },
}

export default () => {
  const form: any = useMemo(() => createForm({
    initialValues:             {
      array: range(100000),
    }
  }), [])

  return (
    <div>
      <FormProvider  form={form}>
        <SchemaField schema={schema} />
        <FormButtonGroup>
          <Submit onSubmit={console.log}>Submit</Submit>
        </FormButtonGroup>
      </FormProvider>
    </div>
  )
}

janryWang · 2023-06-20T03:05:36Z

packages/core/src/models/ArrayField.ts

+
+      const offset = items.length
+      const len = this.value.length + offset
+
+      for (let idx = len - 1; idx > index + offset; idx--) {
+        this.value[idx] = this.value[idx - offset]
+      }
+      for (let idx = 0; idx < offset; idx++) {
+        this.value[index + idx] = items[idx]
+      }
+


详细讲一下这个算法逻辑？为什么splice会慢？

~~先猜的哈 splice 导致了长度变更引起了 array children clean，似乎导致了成吨的 item ob 的销毁和重建，我在 debug 源码找这个调用路径~~

~~for 循环尽可能重用了 item ，在拖拽情况下减少了重建范围~~

splice 会慢的原因是 splice 会遍历访问所有 array items 的 get，从而导致了 reative 中的 proxy get 内部的 createObservable 被多次(value.length)调用
https://github.com/alibaba/formily/blob/formily_next/packages/reactive/src/handlers.ts#L188
从而导致性能问题, 这段代码可以展示问题

const base = Array.from({ length: 10 }).map((_, i) => i); const array = new Proxy(base, { get(t, p) { console.log("p", p) return t[p] } }) console.log(array.splice(1, 2))

output

p splice p length p constructor p 1 p 2 p 3 p 4 p 5 p 6 p 7 p 8 p 9 [ 1, 2 ]

for 循环可以尽量少的减少 get ，从而达到优化效果；
在 10W 数据的 ArrayTable 中进行一次拖拽可以很直观的看到效果，效果拔群

理解了，不过你这个函数最好抽象一下，放到shared里作为一个公共函数来使用吧

我有测试了几次，性能问题不能说是 splice 的锅
而是还在原来 move 方法里的使用方式，两次 splice 导致了 sort 操作的 get 遍历次数增加，因为 sort 操作移动范围本来是有限的，并不需要进行全量的左右移动操作，所以用 for 循环替换掉会效果很好

对于 insert/remove 方法用 splice 是没有问题的

charlzyx · 2023-06-20T11:57:47Z

packages/shared/src/array.ts

+    toIndex > array.length - 1 ||
+    fromIndex > array.length - 1
+  ) {
+    return array


暂时按照这个依据 #3863 (comment)

charlzyx commented Jun 20, 2023

View reviewed changes

janryWang reviewed Jun 20, 2023

View reviewed changes

charlzyx commented Jun 20, 2023

View reviewed changes

charlzyx changed the title ~~perf(core): ArrayField move/insert/remove method optimize~~ perf(core): ArrayField move method optimize Jun 21, 2023

perf: ⚡️ core array move method optimize, move to shared

0a17322

charlzyx force-pushed the pref/core-array-methods branch from fee7600 to 0a17322 Compare June 21, 2023 03:13

janryWang merged commit 3349815 into alibaba:formily_next Jun 25, 2023

charlzyx mentioned this pull request Jun 25, 2023

feat(antd): ArrayTable/ArrayItems sortable impl replace react-sortable-hoc to @dnd-kit #3855

Merged

6 tasks

Uh oh!

Conversation

charlzyx commented Jun 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What have you changed?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

charlzyx Jun 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jun 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

charlzyx commented Jun 20, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

charlzyx Jun 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

charlzyx commented Jun 20, 2023 •

edited

Loading

charlzyx Jun 20, 2023 •

edited

Loading

codecov bot commented Jun 20, 2023 •

edited

Loading

charlzyx Jun 20, 2023 •

edited

Loading