perf: large data with tree grid

[jajugoguma]

Please check if the PR fulfills these requirements

• It's submitted to right branch according to our branching model
• It's right issue type on title
• When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)
• The commit message follows our guidelines
• Tests for the changes have been added (for bug fixes/features)
• Docs have been added/updated (for bug fixes/features)
• It does not introduce a breaking change or has description for the breaking change

Description

• Improved performance when expanding/collapsing in a tree grid of large data.
  • Improved rendering performance by preventing the rendering of child rows that are collapsed and therefore invisible.
    • Previously, it rendered more than the number of collapsed child rows * the number of columns.
  • When collapsing a tree, unobserve the tree property of child rows to avoid unnecessarily performing observable data behavior later on.
• It gave about a 2000x performance improvement.(from about 300000ms to about 150ms)
  • Test environment
    • Large children data: 10000 rows, 50 columns
    • Make all child row to observable by scroll to bottom.
    • M1 Pro, 16GB, Mac OS 13.1, Chrome 113

---

Thank you for your contribution to TOAST UI product. 🎉 😘 ✨

[adhrinae]

개선이 되었으면 좋겠다고 생각하는 부분에 코멘트 남깁니다. 시간이 너무 많이 들거나 어렵다고 생각한다면 패스해도 됩니다.

[adhrinae]

이 함수가 하는 역할이 기존 객체를 getOriginObject를 호출해서 반환된 일반 객체와 어떻게 다르죠?

처음에는 주어진 키만 observable을 해제한다고 이해할 수는 있겠는데, obj.__storage__까지 통으로 날리면 문제가 없나요?

위의 의문이 들다 보니 이 함수는 동작이 부정확해보입니다. 해당 속성을 observerInfoMap에서 제거하는 동작이 보이지 않기 때문입니다. 그러면 점점 불필요한 메모리 낭비가 발생할 수 있습니다.

만약 getOriginObject를 활용하는 방식으로 대체할 수 있을지 아니면 observe 함수를 호출하고 리턴되는 unobserve 함수 등을 활용할 방법은 없을지 궁금하네요.

[jajugoguma]

getOriginObject를 사용하도록 변경했습니다.
그리고 __storage__를 삭제하지 않도록 수정해서 obj를 다시 observable로 변경하지 않으므로 observerInfoMap에 불필요한 데이터가 축적되지 않습니다. 단지 obj에 주어진 속성만 unobservable로 변경하는 정확한 동작을 수행하는 함수로 대체 했습니다.

[adhrinae]

이 로직을 뷰 레이어에서 확인하는게 아니라 collapse를 dispatch할 때 해당 Row가 보여도 되는지 숨겨야 되는지를 미리 가지고 있도록 만들 수 없나요?

뷰 레이어의 복잡도가 불필요하게 올라가는 동작으로 보입니다.

[jajugoguma]

스크럼 때 말씀드린 것과 같이, Row의 hidden 정보를 rawData에 가지고 있어서 이를 확인하는 로직이 수행되게 됩니다. browserStack 등과 같이 저사양 환경에서 확인해본 결과, 동작 수행의 딜레이가 완전히 사라지지는 않았지만 테스트 환경과 마찬가지로 약 2000배 가량의 성능 향상이 있는 것으로 확인 되었습니다.

이와 관련해서는 우선 배포 후, 관련해서 추가 개선이 필요하게 되면 추후 구조 등을 개선하도록 하겠습니다.

[sunyoungBae]

도형 전임님이 남겨주신 의견을 제외하고는 이견 없습니다.

[jwlee1108]

리뷰 완료합니다.

getVisibleStateOfRows 자체 복잡도가 높긴한데 저사양환경에서 테스트 조금만 더 돌려보시고 문제가 없는지 확인 바랍니다. 만약 이슈가 있다면 반환할 때 쓰는 rawData.reduce를 비동기 처리하는 등 한 번 끊어내야할 수도 있겠어요.