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

Poor column drag performance with table sidebar open #1650

Open
dsmmcken opened this issue Nov 17, 2023 · 5 comments · Fixed by deephaven/deephaven-core#4930
Open

Poor column drag performance with table sidebar open #1650

dsmmcken opened this issue Nov 17, 2023 · 5 comments · Fixed by deephaven/deephaven-core#4930
Labels
bug Something isn't working
Milestone

Comments

@dsmmcken
Copy link
Contributor

Description

Table column drag performance can be really poor to "unusable" when a user has a high number of columns and the table sidebar open. It looks like the sidebar is re-drawing on every single grid render/mouse move.

Steps to reproduce

from deephaven import time_table
cols = []
for i in range(500):
    cols.append("column" + str(i) + " = i")
hundreds_of_cols = time_table("PT1S").update_view(cols).reverse()

Using the above table, open the table side bar and go to the organize columns screen. Click on table column in the table and drag it around.

Expected results

Table remains snappy and responsive while dragging.

Actual results
Table is laggy, with frequent pauses that can be greater than 500ms-1000ms. (Which on a ticking table can be near constant).

Additional details and attachments

slow_drag

Versions

Engine Version: 0.31.0
Web UI Version: 0.54.0
Java Version: 21.0.1
Barrage Version: 0.6.0

@dsmmcken dsmmcken added bug Something isn't working triage Issue requires triage labels Nov 17, 2023
@mattrunyon
Copy link
Collaborator

The issue is when the columns are actually swapped, not on render/move. If you drag slowly without swapping the columns it's fine. We trigger a onMovedColumnsChanged as soon as the drag past happens (not on drop). This triggers a re-render in the visibility menu

@dsmmcken
Copy link
Contributor Author

yes, that assessment seems correct
movechanged

@dsmmcken
Copy link
Contributor Author

class VisibilityOrderingBuilder extends Component -> PureComponent, which will reduce renders, but it still sucks when columns actually move.

@dsmmcken
Copy link
Contributor Author

Should probably limit the size of this placeholder.
image

dsmmcken added a commit that referenced this issue Nov 20, 2023
dsmmcken added a commit that referenced this issue Nov 20, 2023
Minor drop in the bucket against #1650, reduces at least some of the
re-rendering.
@vbabich vbabich added this to the November 2023 milestone Nov 21, 2023
@vbabich vbabich removed the triage Issue requires triage label Nov 21, 2023
@dsmmcken
Copy link
Contributor Author

This might partly be dndkit performance:

clauderic/dnd-kit#1194 (comment)

From an August issue posted on dnd-kit, they have a refactored experimental branch that should help reduce re-renders. No idea when it will land though, and may not entirely fix it.

Current recommended mitigation is to keep the table drag pending until dropped so it doesn't trigger the list to be re-ordered while it is dragging past each column. Be sure to check that a browser disconnect/exit while dragging doesn't leave you in a broken state after this change (as I believe that was the reason for the current behaviour or committing the drag position immediately).

mofojed pushed a commit to deephaven/deephaven-core that referenced this issue Dec 11, 2023
Release notes https://github.com/deephaven/web-client-ui/releases/tag/v0.56.0

# [0.56.0](deephaven/web-client-ui@v0.55.0...v0.56.0) (2023-12-11)


### Bug Fixes

* add right margin to <Button kind='inline'/> using icons ([#1664](deephaven/web-client-ui#1664)) ([fd8a6c6](deephaven/web-client-ui@fd8a6c6))
* adjust filter bar colour ([#1666](deephaven/web-client-ui#1666)) ([4c0200e](deephaven/web-client-ui@4c0200e))
* convert organize columns component to purecomponent ([#1653](deephaven/web-client-ui#1653)) ([8ddc114](deephaven/web-client-ui@8ddc114)), closes [#1650](deephaven/web-client-ui#1650)
* Default to `Skip` operation instead of `Sum` operation ([#1648](deephaven/web-client-ui#1648)) ([6083173](deephaven/web-client-ui@6083173)), closes [#1355](deephaven/web-client-ui#1355) [#1355](deephaven/web-client-ui#1355)
* Fix button snapshots ([#1655](deephaven/web-client-ui#1655)) ([c0cc966](deephaven/web-client-ui@c0cc966))
* popper blur in styleguide ([#1672](deephaven/web-client-ui#1672)) ([6fa2204](deephaven/web-client-ui@6fa2204))
* Unable to delete selected rows in some input tables ([#1678](deephaven/web-client-ui#1678)) ([1e71550](deephaven/web-client-ui@1e71550)), closes [#1677](deephaven/web-client-ui#1677)


### Features

* Add embed-widget ([#1668](deephaven/web-client-ui#1668)) ([1b06675](deephaven/web-client-ui@1b06675)), closes [#1629](deephaven/web-client-ui#1629)
* forward and back button for organize column search ([#1641](deephaven/web-client-ui#1641)) ([89f2be5](deephaven/web-client-ui@89f2be5)), closes [#1529](deephaven/web-client-ui#1529)
* Tables that have names starting with underscore do not auto-launch from console ([#1656](deephaven/web-client-ui#1656)) ([21131fe](deephaven/web-client-ui@21131fe)), closes [#1549](deephaven/web-client-ui#1549) [#1410](deephaven/web-client-ui#1410)
* theme fontawesome icon size wrapped in spectrum icons ([#1658](deephaven/web-client-ui#1658)) ([2aa8cef](deephaven/web-client-ui@2aa8cef))
* Theme Selector ([#1661](deephaven/web-client-ui#1661)) ([5e2be64](deephaven/web-client-ui@5e2be64)), closes [#1660](deephaven/web-client-ui#1660)
* Theming - Bootstrap ([#1603](deephaven/web-client-ui#1603)) ([88bcae0](deephaven/web-client-ui@88bcae0))
* Theming - Inline svgs ([#1651](deephaven/web-client-ui#1651)) ([1e40d3e](deephaven/web-client-ui@1e40d3e))
* View cell contents in context menu ([#1657](deephaven/web-client-ui#1657)) ([90b7517](deephaven/web-client-ui@90b7517)), closes [#1605](deephaven/web-client-ui#1605)


### BREAKING CHANGES

* Bootstrap color variables are now predominantly hsl based. SCSS will need to be updated accordingly. Theme providers are needed to load themes.
* Tables assigned to variable beginning with "_" will not open automatically even if "Auto Launch Panels" is checked.

Co-authored-by: deephaven-internal <[email protected]>
@mattrunyon mattrunyon reopened this Dec 11, 2023
@mofojed mofojed modified the milestones: November 2023, January 2024 Jan 9, 2024
@mofojed mofojed modified the milestones: January 2024, February 2024 Feb 13, 2024
@mofojed mofojed modified the milestones: February 2024, March 2024 Mar 12, 2024
@mofojed mofojed modified the milestones: March 2024, April 2024 Apr 12, 2024
@mofojed mofojed modified the milestones: April 2024, May 2024 May 13, 2024
@mofojed mofojed modified the milestones: May 2024, June 2024 Jun 4, 2024
@mofojed mofojed removed their assignment Jul 8, 2024
@vbabich vbabich modified the milestones: June 2024, Backlog Jul 9, 2024
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