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

Web UI becomes frozen #3941

Open
Tracked by #5927
jcferretti opened this issue Jun 5, 2023 · 4 comments
Open
Tracked by #5927

Web UI becomes frozen #3941

jcferretti opened this issue Jun 5, 2023 · 4 comments
Assignees
Labels
bug Something isn't working jsapi
Milestone

Comments

@jcferretti
Copy link
Member

"Just" [TM] running this, the web UI freezes:

from deephaven import time_table
t = time_table("PT1S").update("A=new java.math.BigDecimal(java.math.BigInteger.valueOf(i)).setScale(10000)")
t2 = t.update("B = A.pow(2) / (A + 1)")

The server meanwhile doesn't look unhappy:

server_1  | 2023-06-05T23:34:33.369Z | r-Scheduler-Serial-1 |  INFO | .p.PythonDeephavenSession | Evaluating command: from deephaven import time_table
server_1  | t = time_table("PT1S").update("A=new java.math.BigDecimal(java.math.BigInteger.valueOf(i)).setScale(10000)")
server_1  | t2 = t.update("B = A.pow(2) / (A + 1)")
server_1  | 2023-06-05T23:34:35.340Z | EFAULT.refreshThread |  INFO | e.u.i.PeriodicUpdateGraph | Minimal Update Graph Processor cycle times: 27.576ms / 3 cycles = 9.192ms/cycle average), safePointTime=0ms
server_1  | 2023-06-05T23:34:38.342Z | EFAULT.refreshThread |  INFO | e.u.i.PeriodicUpdateGraph | Minimal Update Graph Processor cycle times: 28.520ms / 3 cycles = 9.507ms/cycle average), safePointTime=0ms
server_1  | 2023-06-05T23:34:41.348Z | EFAULT.refreshThread |  INFO | e.u.i.PeriodicUpdateGraph | Minimal Update Graph Processor cycle times: 31.558ms / 3 cycles = 10.519ms/cycle average), safePointTime=1ms
server_1  | 2023-06-05T23:34:44.344Z | EFAULT.refreshThread |  INFO | e.u.i.PeriodicUpdateGraph | Minimal Update Graph Processor cycle times: 27.811ms / 3 cycles = 9.270ms/cycle average), safePointTime=0ms
server_1  | 2023-06-05T23:34:48.345Z | EFAULT.refreshThread |  INFO | e.u.i.PeriodicUpdateGraph | Minimal Update Graph Processor cycle times: 32.156ms / 4 cycles = 8.039ms/cycle average), safePointTime=0ms
server_1  | 2023-06-05T23:34:53.340Z | EFAULT.refreshThread |  INFO | e.u.i.PeriodicUpdateGraph | Minimal Update Graph Processor cycle times: 25.168ms / 5 cycles = 5.034ms/cycle average), safePointTime=0ms
server_1  | 2023-06-05T23:34:58.342Z | EFAULT.refreshThread |  INFO | e.u.i.PeriodicUpdateGraph | Minimal Update Graph Processor cycle times: 28.772ms / 5 cycles = 5.754ms/cycle average), safePointTime=0ms

You may ask "why would anybody want a BigDecimal with 10,000 precision?". I was just looking for some table operation that would consume a lot of CPU and take a long time in the UGP for updating. Turns out the UGP is happy happy, the web UI not so much.

Capture_136

@jcferretti jcferretti added bug Something isn't working triage labels Jun 5, 2023
@dsmmcken
Copy link
Contributor

dsmmcken commented Jun 6, 2023

It is spending a lot of time in here, for getting the display value:
image

stack repeats
image

a lot. presumably for each row in viewport (I think 3x whatever is visible or something?)
image

That's all gonna get memoized, but it is locking in the initial call.

Then if you wait long enough, iris grid makes a similar set of calls a second time for determine the cell text color... (we probably can do something smarter there, or at least also memo it)

image

@mofojed mofojed removed their assignment Jun 6, 2023
@niloc132 niloc132 self-assigned this Jun 6, 2023
@niloc132
Copy link
Member

niloc132 commented Jun 6, 2023

I'm tentatively assigning this to myself and marking it as part of the other preview work i would be doing, but this is pretty fringe - "the engine should tell the UI to display bigdecimals with 10000 digits of precision" is costly on the render side too. Presumably a preview feature could make the decision for the user that their precision isn't important after all, so that the barrage payloads aren't large, and the render time isn't terrible.

Without a specific use case though, I'm interpreting this as "very low priority".

@niloc132 niloc132 added jsapi and removed triage labels Jun 6, 2023
@niloc132 niloc132 added this to the Backlog milestone Jun 6, 2023
@niloc132
Copy link
Member

niloc132 commented Jun 6, 2023

#188

@mofojed
Copy link
Member

mofojed commented Jun 6, 2023

The UI is calling valueOf from within the colorForCell method when comparing the value with a number:

          if ((value as number) > 0) {
            assertNotNull(theme.positiveNumberColor);
            return theme.positiveNumberColor;
          }
          if ((value as number) < 0) {
            assertNotNull(theme.negativeNumberColor);
            return theme.negativeNumberColor;
          }

I'm not sure why that triggers a valueOf call? Since that returns a String:

color-value-of

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

No branches or pull requests

4 participants